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

[Diego Biurrun]

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.

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

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;

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.

> +    return (1<<count) - 1 + value;

I find spaces around the << more readable.

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

overridden

> +        /* 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

> +        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.

Diego