[FFmpeg-devel] [PATCH]Fix for issue694. Dirac A/V sync loss

[Anuradha Suraparaju]

Hi,

On Thu, 2008-11-06 at 11:05 +0100, Diego Biurrun wrote:
> On Thu, Nov 06, 2008 at 01:49:55PM +1100, Anuradha Suraparaju wrote:
> > 
> > On Sat, 2008-10-18 at 21:50 +0200, Diego Biurrun wrote:
> > > Hi Anuradha, I wanted to point out the following bug report we received
> > > to you:
> > > 
> > > https://roundup.mplayerhq.hu/roundup/ffmpeg/issue694
> > > 
> > > A/V sync is lost with Dirac.  It's likely an issue in your code.
> > 
> > I've attached a patch for the Dirac A/V sync loss. The patch modifies 3
> > files.
> 
> some nits below
> 
> > --- libavcodec/dirac_parser.c	(revision 15783)
> > +++ libavcodec/dirac_parser.c	(working copy)
> > @@ -34,40 +35,298 @@
> > -static int find_frame_end(ParseContext *pc, const uint8_t *buf, int buf_size)
> > +typedef struct DiracParseContext
> >  {
> 
> The brace is mostly kept on the same line as the typedef struct in the
> rest of FFmpeg, same below.
> 

Fixed.

> > +static int find_frame_end(DiracParseContext *pc, const uint8_t *buf, int buf_size)
> > +{
> 

Fixed

> Please break long lines in function declarations like the rest of the
> file does, same below.
> 
> > +    if (!pc->is_synced)
> > +    {
> > +        for (i = 0; i < buf_size; i++) {
> > +            state = (state << 8) | buf[i];
> > +            if (state == DIRAC_PARSE_INFO_PREFIX) {
> 
> Inconsistent brace placement after if; please keep it on the same line
> like the rest of the file does, same below.
> 
> > +                    pc->state=-1;
> > +                    return i+pc->header_bytes_needed;
> 

Fixed. 

> This would look more readable with a bit of whitespace around = and +.
> 
> > +    pu->next_pu_offset = (start[5] << 24) +
> > +                         (start[6] << 16) +
> > +                         (start[7] << 8)  +
> > +                         start[8];
> > +
> > +    pu->prev_pu_offset = (start[9] << 24)  +
> > +                         (start[10] << 16) +
> > +                         (start[11] << 8)  +
> > +                         start[12];
> 
> This could be aligned.
> 

Fixed. 

> > +    return (1<<count) - 1 + value;
> 
> I find spaces around the << more readable.
> 

Fixed. 

> > +    /* Check if the default was over-ridden while encoding */
> 
> overridden
> 

Fixed. 

> > +        /* Found a possible frame start and a  possible frame end */
> 
> > +         * is equal  to the next parse offset of the current parse unit then
> 
> two spaces
> 

Fixed.

> > +        pc->dirac_unit = av_realloc( pc->dirac_unit, pc->dirac_unit_size + pu.next_pu_offset);
> > +        memcpy(pc->dirac_unit + pc->dirac_unit_size, pc->buffer + pc->buffer_size - 13 - pu1.prev_pu_offset,  pu.next_pu_offset);
> 
> These are loooong lines, same in other places.
> 
> > +        /* Get the picture number to set the pts and dts*/
> 
> dts */
> 
> > +        /* Assume that data has been packetised into an encapsulation unit */
> 
> PacketiZed, American English is preferred.

Fixed.

I've attached a modified patch to this email.

Regards,
Anuradha
-------------- next part --------------
A non-text attachment was scrubbed...
Name: issue694_libdirac_libschroedinger_svn_15785.diff
Type: text/x-patch
Size: 13359 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081107/c33646e1/attachment.bin>