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

[Don Moir]

----- Original Message ----- 
From: "Marton Balint" <cus at passwd.hu>
To: "FFmpeg development discussions and patches" <ffmpeg-devel at ffmpeg.org>
Sent: Thursday, May 30, 2013 5:34 PM
Subject: Re: [FFmpeg-devel] [PATCH 1/2] ffplay: do not allow wider window than 16383


>
>
> 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.

Yeah I know, but it has happened a few times where display aspect is just wrong. I think ffmpeg should just leave it to what it 
thinks it should be. So just asking what your thoughts are on that from an app point of view. I have not found a video that has a 
strange aspect like that, that is actually correct. But have just a few videos where aspect is off like that.

>> 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.

Well for me personally and not just ffplay but other players as well, when it goes offscreen I might just want to resize it and not 
go full screen and it can be a pain in butt moving window and resizing it. Some otheer players do this and then the controls are 
gone offscreen too. Most other players have a setting to control this though.


--------------------------------------------------------------------------------


> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>