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

[Maarten Lankhorst]

Hey Christian,

On 08/30/2011 12:05 PM, Christian König wrote:
> 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.
Ah sure that makes sense. Do you think it's best to reject in that case?
>> 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...
>
Good idea, I should probably test that. I think it might hang
the gpu as it is now when macroblocks or motion vectors
go outside the picture, I've had it a few times on accident.


~Maarten