[Mesa-dev] [PATCH 3/3] xorg/xvmc: Add missing call to set_picture_parameters

[Christian König]

Am Dienstag, den 30.08.2011, 00:11 +0200 schrieb Maarten Lankhorst:
> Hey Christian,
> 
> On 08/29/2011 10:14 AM, Christian König wrote:
> > Am Sonntag, den 28.08.2011, 19:13 +0200 schrieb Maarten Lankhorst:
> >> Hey,
> >>
> >> On 08/28/2011 05:04 PM, Christian König wrote:
> >>> Hi Maarten,
> >>>
> >>> could you put this into SetDecoderStatus instead? This makes the picture
> >>> structure also available in end_frame.
> >> Erm it doesn't make sense there. It might make sense to move it to after
> >> the begin_frame though, but with interlaced frames I don't see why you
> >> would want to move it into SetDecoderStatus, since you could have interlaced
> >> frames with different picture_structures.
> > Oh, what I wanted to say is: Remember the picture structure in use when
> > begin_frame is called and just set it to the same value when end_frame
> > is called. Also having a end_frame/begin_frame when the picture
> > structure changes from top to bottom (or the other way around) couldn't
> > hurt also.
> >
> The nouveau decoder doesn't need to remember the picture structure,
> it only needs it during the render call. If vl_mpeg12_decoder needs it,
> wouldn't it make more sense to remember it in the vl_mpeg12_buffer?
I'm tending to say no, because I think the basic idea behind a state
tracker is that it should remember the state of the interface, instead
of the driver remembering it (basically just doing what the name says). 

I think the question we should ask isn't if one driver or another needs
something, but more if one or another behaviour is specific to an
interface or not. So when an interface theoretically allows a behaviour
(even if it's completely invalid like changing the picture structure in
the middle of a frame) it is the job of the state tracker to come up
with a proper solution to handle this behaviour.

> Also I think it's safe to assume that the picture_structure won't change,
> since the mpeg spec assumes its constant for the entire frame.
I won't assume anything about the correctness of the stream. The mplayer
guys have made quite a collection of streams which can crash the
decoder: http://samples.mplayerhq.hu/MPEG2/

It's including stuff like changing the resolution of the picture in the
middle of a stream, corrupt streams where an macroblock address is
suddenly way out of the picture etc..

You should test that collection with your decoder and even if there is
no way to correctly decode an invalid stream, it at least shouldn't
crash, go into an endless loop...

Regards,
Christian.