[FFmpeg-devel] [RFC] support for Cam_PAL_D1_inter_Field_25fps_2.5MBS.mp4

[Michael Niedermayer]

On Fri, Mar 21, 2008 at 01:50:47PM +0100, Reimar D?ffinger wrote:
> On Thu, Mar 20, 2008 at 07:26:20PM +0100, Michael Niedermayer wrote:
> > On Thu, Mar 20, 2008 at 03:43:34PM +0100, Reimar D?ffinger wrote:
> > > Hello,
> > > I made a patch that works for:
> > > http://samples.mplayerhq.hu/V-codecs/h264/PAFF/Cam_PAL_D1_inter_Field_25fps_2.5MBS.mp4
> > > I think this is probably the wrong approach and instead we need an
> > > AVParser for H.264 in mov, but I am not entirely sure, so please
> > > have a look at attached patch.
> > 
> > Could you explain in english words what the problem with that file is
> > and what your patch does? I could guess that its something with multiple
> > fields in a packet ...
> 
> The problem with that file is,
> 1) only I-frames are displayed, since the code never finds a
> complementary field, which I think is due to the bottom field being in
> the same packet, so slice number is != 0, so the whole
> check-for-complete-frame code does not run.
> 2) Probably related to this, also the reference pictures used are the
> wrong ones, as can be seen when making FFmpeg always output a picture
> even if it thinks one field is still missing.
> 
> > > Index: libavcodec/h264.c
> > > ===================================================================
> > > --- libavcodec/h264.c	(revision 12478)
> > > +++ libavcodec/h264.c	(working copy)
> > > @@ -3884,6 +3884,7 @@
> > >  
> > >      if((s->flags2 & CODEC_FLAG2_CHUNKS) && first_mb_in_slice == 0){
> > >          h0->current_slice = 0;
> > > +        h0->block_available_fields = 0;
> > 
> > It does not seem that we have such a variable in the context, and your
> > patch doesnt add one ...
> > 
> > 
> > >          if (!s0->first_field)
> > >              s->current_picture_ptr= NULL;
> > >      }
> > > @@ -4003,7 +4004,20 @@
> > >          s->picture_structure= PICT_FRAME;
> > >      }else{
> > >          if(get_bits1(&s->gb)) { //field_pic_flag
> > > -            s->picture_structure= PICT_TOP_FIELD + get_bits1(&s->gb); //bottom_field_flag
> > > +            int next_picture_structure = PICT_TOP_FIELD + get_bits1(&s->gb); //bottom_field_flag
> > 
> > > +            int mask = next_picture_structure == PICT_TOP_FIELD ? 1 : 2;
> > 
> > this is just mask = next_picture_structure
> 
> Updated, though as said I doubt it is quite the right way to do it, I am
> mostly hoping for someone else to come up with a better solution after
> the problem is truly clear (which I hope this patch will help to do).
> 
> Greetings,
> Reimar D?ffinger

> Index: libavcodec/h264.c
> ===================================================================
> --- libavcodec/h264.c	(revision 12478)
> +++ libavcodec/h264.c	(working copy)
> @@ -3884,6 +3884,7 @@
>  
>      if((s->flags2 & CODEC_FLAG2_CHUNKS) && first_mb_in_slice == 0){
>          h0->current_slice = 0;
> +        h0->block_available_fields = 0;
>          if (!s0->first_field)
>              s->current_picture_ptr= NULL;
>      }
> @@ -4003,7 +4004,19 @@
>          s->picture_structure= PICT_FRAME;
>      }else{
>          if(get_bits1(&s->gb)) { //field_pic_flag
> -            s->picture_structure= PICT_TOP_FIELD + get_bits1(&s->gb); //bottom_field_flag
> +            int next_picture_structure = PICT_TOP_FIELD + get_bits1(&s->gb); //bottom_field_flag
> +            if(h0->block_available_fields == (next_picture_structure ^ 3)) {
> +                if(!s->dropable) {
> +                    h0->prev_poc_msb = h0->poc_msb;
> +                    h0->prev_poc_lsb = h0->poc_lsb;
> +                    execute_ref_pic_marking(h0, h0->mmco, h0->mmco_index);
> +                }
> +                h0->current_slice = 0;
> +                h0->block_available_fields |= next_picture_structure;
> +                return 1;
> +            }
> +            h0->block_available_fields |= next_picture_structure;
> +            s->picture_structure = next_picture_structure;
>          } else {
>              s->picture_structure= PICT_FRAME;
>              h->mb_aff_frame = h->sps.mb_aff;
> @@ -7442,6 +7455,7 @@
>  #endif
>      if(!(s->flags2 & CODEC_FLAG2_CHUNKS)){
>          h->current_slice = 0;
> +        h->block_available_fields = 0;
>          if (!s->first_field)
>              s->current_picture_ptr= NULL;
>      }

> @@ -7600,6 +7614,10 @@
>               * NAL unit stuff to context 0 and restart. Note that
>               * rbsp_buffer is not transfered, but since we no longer
>               * run in parallel mode this should not be an issue. */
> +            if(context_count) {
> +                execute_decode_slices(h, context_count);
> +                context_count = 0;
> +            }

What values do context_count and h->max_contexts have here? and why
does the 
if(context_count == h->max_contexts) {
above not trigger?

max_contexts should be 1 and context_count should be 1 as well i think

and could you dump the NAL unit types each frame has? 
(debug= FF_DEBUG_STARTCODE IIRC)

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080326/f927c86c/attachment.pgp>