[Libva] mpeg4 decoding

[Xiang, Haihao]

On Mon, 2015-01-12 at 21:38 +0000, Varga, Michael wrote:
> Haihao,
> 
> I use modulo_time_base when reconstructing the header VideoObjectPlane header.

Does HW need the VOP header for decode ?

>  When modulo_time_base is left blank frames do not decode correctly. 
> From your comment your refering to vop_time_increment? 

No, I didn't refer to vop_time_increment or modulo_time_base. I just
wonder it is really needed to add a new field in
VAPictureParameterBufferMPEG4 or not.


> For gpu acceleration vop_time_increment can be set to "0" AFAIK.
> 
> MPEG 4 spec:
> modulo_time_base: This value represents the local time base in one second resolution units (1000 milliseconds).
> It consists of a number of consecutive '1' followed by a '0'. Each '1' represents a duration of one second that have
> elapsed. For I-, S(GMC)-, and P-VOPs of a non scalable bitstream and the base layer of a scalable bitstream, the
> number of '1's indicate the number of seconds elapsed since the synchronization point marked by time_code of
> the previous GOV header or by modulo_time_base of the previously decoded I-, S(GMC)-, or P-VOP, in decoding
> order. For B-VOP of a non scalable bitstream and a base layer of a scalable bitstream, the number of '1's indicates
> the number of seconds elapsed since the synchronization point marked in the previous GOV header, or I-VOP,
> S(GMC)-VOP, or P-VOP, in display order. For I-, P-, or B-VOPs of enhancement layer of scalable bitstream, the
> number of '1's indicate the number of seconds elapsed since the synchronization point marked in the previous
> GOV header, I-VOP, P-VOP, or B-VOP, in display order.
> 
> vop_time_increment: This value represents the absolute vop_time_increment from the synchronization point
> marked by the modulo_time_base measured in the number of clock ticks. It can take a value in the range of
> [0,vop_time_increment_resolution). The number of bits representing the value is calculated as the minimum
> number of unsigned integer bits required to represent the above range. The local time base in the units of seconds
> is recovered by dividing this value by the vop_time_increment_resolution.
> 
> 
> Thanks
> Michael
> 
> -----Original Message-----
> From: Xiang, Haihao [mailto:haihao.xiang at intel.com] 
> Sent: Wednesday, January 07, 2015 11:51 PM
> To: Varga, Michael
> Cc: Emil Velikov; Gwenole Beauchesne; libva at lists.freedesktop.org
> Subject: Re: [Libva] mpeg4 decoding
> 
> 
> Hi Michael,
> 
>  I am not familiar with MPEG4, I think modulo_time_base is used to calculate time stamp etc, is modulo_time_base really required for HW decode ?
> 
> Thanks
> Haihao
> 
> > I only really need modulo_time_base. It would appear from my tests vop_time_increment is not needed. Could modulo_time_base be added?
> > 
> > The core issue I am having is I need the modulo_time_base value. I noticed Gwenole wrote a reference driver which wraps vdpau. Gwenole's code has a fix to compute modulo_time_base. I cannot use code from Gwenole's driver due to licensing issues, his code is GPL and MESA is MIT.
> > 
> > -----Original Message-----
> > From: Emil Velikov [mailto:emil.l.velikov at gmail.com]
> > Sent: Saturday, November 08, 2014 9:58 AM
> > To: Varga, Michael; Gwenole Beauchesne
> > Cc: emil.l.velikov at gmail.com; libva at lists.freedesktop.org
> > Subject: Re: [Libva] mpeg4 decoding
> > 
> > Hi guys.
> > 
> > Don't mean to be picky or anything but won't this change cause a non-backward compat ABI/API breakage ?
> > 
> > While adding modulo_time_base might be ok (highly dependant on the implementation which may use the vop_fields union without sanitising it), vop_time_increment is definitely going to cause issues.
> > 
> > 
> > Cheers,
> > Emil
> > 
> > On 31/10/14 14:32, Varga, Michael wrote:
> > > Hi Gwenole,
> > > 
> > > I double checked vop_time_increment and its not strictly needed, 
> > > setting this value to 0 works but if you'll add it I'll use anyway. 
> > > My patch is below. I declared modulo_time_base with 4 bits, the 
> > > MPEG4 spec does not specify an upper bound for the size of this 
> > > variable. If you think this is too big I can reduce it. Also, is 
> > > there anything you think I should know about your process for submitting patches? Thanks!
> > > :)
> > > 
> > > 
> > > From 44aeb5d81789c42404d6a2398ef06699aabc6d73 Mon Sep 17 00:00:00 
> > > 2001
> > > From: Michael Varga <Michael.Varga at amd.com>
> > > Date: Fri, 31 Oct 2014 08:58:34 -0500
> > > Subject: [PATCH] va: added two variables to
> > > VAPictureParameterBufferMPEG4
> > > 
> > > Added modulo_time_base and vop_time_increment so the VOP header may 
> > > be reconstructed if it has been truncated.
> > > 
> > > Signed-off-by: Michael Varga <Michael.Varga at amd.com>
> > > ---
> > >  va/va.h | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/va/va.h b/va/va.h
> > > index 01694a9..c21770c 100644
> > > --- a/va/va.h
> > > +++ b/va/va.h
> > > @@ -1312,12 +1312,14 @@ typedef struct _VAPictureParameterBufferMPEG4
> > >              unsigned int intra_dc_vlc_thr              : 3; 
> > >              unsigned int top_field_first               : 1; 
> > >              unsigned int alternate_vertical_scan_flag  : 1;
> > > +            unsigned int modulo_time_base              : 4;
> > >          } bits;
> > >          unsigned int value;
> > >      } vop_fields;
> > >      unsigned char vop_fcode_forward;
> > >      unsigned char vop_fcode_backward;
> > >      unsigned short vop_time_increment_resolution;
> > > +    unsigned short vop_time_increment;
> > >      /* short header related */
> > >      unsigned char num_gobs_in_vop;
> > >      unsigned char num_macroblocks_in_gob;
> > > 
> > 
> > _______________________________________________
> > Libva mailing list
> > Libva at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/libva
> 
>