[Ffmpeg-devel] [PATCH] Auto-resize ffplay video

[Michael Niedermayer]

Hi

On Sat, Dec 30, 2006 at 07:44:29PM -0600, Steven Sheehy wrote:
> Michael Niedermayer wrote:
> >Hi
> >
> >On Fri, Dec 29, 2006 at 05:07:14PM -0600, Steven Sheehy wrote:
> >>Michael Niedermayer wrote:
> >>>Hi
> >>>
> >>>On Tue, Dec 26, 2006 at 03:12:38PM -0600, Steven Sheehy wrote:
> >>>>On Sat, Dec 23, 2006 at 15:20:36 CET 2006, Michael Niedermayer wrote:
> >>>>>seperate issue seperate patch please
> >>>>I could've characterized this patch more generically by saying it 
> >>>>resolves the improper resizing of the video window, if you prefer. In 
> >>>>addition, if I were to make them separate they would still depend on 
> >>>>each other since the patch to auto-resize the output modifies text 
> >>>>before the exiting from full screen bug. Nevertheless, here's the patch 
> >>>>for just this issue:
> >>>>
> >>>[mangled unuseable patch]
> >>>
> >>>>>screen_width / screen_height should either be set to the dimensions of
> >>>>>the video or should be at 0 by default and then be overridden by
> >>>>>the video w/h if they are 0 where used
> >>>>I've tried this, but I've found that if you pass a width and height of 
> >>>>0 to the initial SDL_SetVideoMode(), it causes the video window to be 
> >>>>maximized and future SDL_SetVideoMode()'s to be ignored (SDL bug?). 
> >>>>screen_width / screen_height cannot be set to the video dimensions at 
> >>>>declaration time or in main() since they can only be determined at a 
> >>>>much later point in the program when alloc_picture() is called. If have 
> >>>>any suggestions to overcome these issues, I'd be grateful.
> >>>what about calling SDL_SetVideoMode() after av_find_stream_info() ?
> >>>
> >>I followed your advice and basically moved the initial call to 
> >>SDL_SetVideoMode() from main() to decode_thread() after 
> >>av_find_stream_info(). This works perfectly for me, but I'm not sure if 
> >>this is how you want it. Let me know if I need to tweak it more.
> >
> >hmm, maybe putting the code into stream_component_open() would be better
> >sample_rate for audio is also set there, so w/h seems to fit pretty well
> >also stream_cycle_channel() calls stream_component_open() and if the
> >w/h changes should update the window size
> >
> >of course the code could also be put into its own function which would
> >then be called by stream_component_open() ...
> >
> >[...]
> 
> I see you decided just to write your own solution.
> 
> In testing your approach, I've discovered some bugs:
> 
> * Crashes when playing non-video files

fixed


> * Crashes if I specify a negative value for -x option (it is better to 
> check if (screen_width <= 0) instead of just if (screen_width)).

fixed


> * When -y option is specified by itself it is ignored.
> * When -x option is specified by itself the height is not the default 
> height of the video.

and it should not be the "default" height of 480 but rather the one which
preserves the aspect ratio


> * Returning from full-screen ignores any resizing the user did.

not reproduceable, resizing behaves pretty random here even without
fullscreen, not sure if that was caused by my changes or if it was broken
before
also try -x 10 -y 10 and then switch to fullscreen (10x10 scaled to
fullscreen, this one wasnt caused by me messing around with the code)


> 
> As I found out and you can probably see, this resizing issue is a bit more 
> troublesome than it at first seems. :)

yes

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20061231/2cff7426/attachment.pgp>