[FFmpeg-devel] [PATCH 1/2] ffplay: do not allow wider window than 16383

[Marton Balint]

On Thu, 30 May 2013, Don Moir wrote:

>
> ----- Original Message ----- From: "Reimar Döffinger" 
> <Reimar.Doeffinger at gmx.de>
> To: "FFmpeg development discussions and patches" <ffmpeg-devel at ffmpeg.org>
> Sent: Wednesday, May 29, 2013 6:31 PM
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] ffplay: do not allow wider window 
> than 16383
>
>
>> On Wed, May 29, 2013 at 11:38:50PM +0200, Marton Balint wrote:
>>> SDL surface pitch is 16bit, to avoid possible overflows, we limit the 
>>> window
>>> width to 16383. Fixes ticket #2428.
>>> 
>>> Signed-off-by: Marton Balint <cus at passwd.hu>
>>> ---
>>>  ffplay.c | 12 ++++++++----
>>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/ffplay.c b/ffplay.c
>>> index 80c3091..2b85c14 100644
>>> --- a/ffplay.c
>>> +++ b/ffplay.c
>>> @@ -1070,7 +1070,7 @@ static int video_open(VideoState *is, int 
>>> force_set_video_mode, VideoPicture *vp
>>>      if (screen && is->width == screen->w && screen->w == w
>>>         && is->height== screen->h && screen->h == h && 
>>> !force_set_video_mode)
>>>          return 0;
>>> -    screen = SDL_SetVideoMode(w, h, 0, flags);
>>> +    screen = SDL_SetVideoMode(FFMIN(16383, w), h, 0, flags);
>> 
>> Are you sure we do not need to update something to our hacked width?
>> In particular, wouldn't the if right above possibly result in us
>> reinitializing SDL over and over in certain cases?
>
> Anyone think that the display aspect ratio in wide.avi is so off base it 
> should be ignored ? Not sure what a lower and higher end for display aspect 
> ratio should be but maybe between 1/5 and 5. The display aspect is 53 in 
> wide.avi.

I don't know, just because an aspect ratio is uncommon, I don't think we 
should ignore it.

> Also I think you should put some upper limit on display width/height by 
> default for ffplay if you don't have the actual screen dimensions like maybe 
> 800x600. User can resize if needed. No one wants to see the video shoot 
> offscreen.

That may not be true, for example I quite like the idea that the initial 
ffplay window contains the same amount of lines as the video itself, so 
for an 1:1 aspect ratio video it will try to do a per-pixel display. If my 
screen is smaller than the video, and I want to see the whole stuff, I 
just go to fullscreen.

Regards,
Marton