[FFmpeg-devel] [PATCH]Fix bug in setting the pts when decoding or transcoding Dirac video wrapped in MPEG-TS using libschroedingerdec.c

[Reimar Döffinger]

On Mon, Nov 29, 2010 at 02:03:51PM +1100, Anuradha Suraparaju wrote:
> +    int64_t  *pkttag;

Why two spaces?

> @@ -234,6 +246,11 @@
>              if (SCHRO_PARSE_CODE_IS_PICTURE(enc_buf->data[4]) &&
>                  SCHRO_PARSE_CODE_NUM_REFS(enc_buf->data[4]) > 0)
>                  avccontext->has_b_frames = 1;
> +
> +            pkttag = av_mallocz(sizeof(int64_t));

sizeof(*pkttag) possibly.
Not sure about the performance impact, it's not exactly efficient,
also for memory fragmentation.

> -                if (frame)
> +                if (frame) {
> +                    FfmpegSchroFrame *p_frame = av_mallocz(sizeof(FfmpegSchroFrame));

Same comments as above.

> @@ -285,30 +309,33 @@
>      } while (outer);
>  
>      /* Grab next frame to be returned from the top of the queue. */
> -    frame = ff_dirac_schro_queue_pop(&p_schro_params->dec_frame_queue);
> +    f = ff_dirac_schro_queue_pop(&p_schro_params->dec_frame_queue);
>  
> -    if (frame) {
> +    if (f) {
>          memcpy(p_schro_params->dec_pic.data[0],
> -               frame->components[0].data,
> -               frame->components[0].length);
> +               f->frame->components[0].data,
> +               f->frame->components[0].length);
>  
>          memcpy(p_schro_params->dec_pic.data[1],
> -               frame->components[1].data,
> -               frame->components[1].length);
> +               f->frame->components[1].data,
> +               f->frame->components[1].length);
>  
>          memcpy(p_schro_params->dec_pic.data[2],
> -               frame->components[2].data,
> -               frame->components[2].length);
> +               f->frame->components[2].data,
> +               f->frame->components[2].length);

Adding a local "frame" variable would reduce the diff a bit.

> +        if (f->p_tag) {
> +            picture->reordered_opaque = *((int64_t *)f->p_tag->value);
> +        }

That's a {} and () more than really necessary.
Otherwise I can't really comment on whether it is correct/works
as intended/whatever.