[vlc-devel] [PATCH] Enable aspectratio parameter in mozillaplugin

[Vicente Jiménez]

On Thu, Jun 11, 2009 at 9:59 PM, <jean-paul.saman at planet.nl> wrote:
> On Wed, Jun 10, 2009 at 1:29 PM, Vicente Jiménez<googuy at gmail.com> wrote:
>> Thanks for the review!
>>
>> I've suposed it was C++ code because of the file extension. I was
>> looking for similar parameter operations in the code but found none
>> and I supposed that the concatenation could be done with +. Thanks for
>> the clarifications.
>>
>> You mean to do:
>>            ppsz_argv[ppsz_argc++] = "--aspect-ratio";
>>            ppsz_argv[ppsz_argc++] = argv[i];
>>
>> Is this correct?
>>
> No it isn't correct. This patch opens a can of worms.
>
> The mozilla plugin has already support for specifying an aspect ration
> (vlc.video.aspectRatio). Please read the documentation on
> http://wiki.videolan.org/Documentation:WebPlugin#Building_HTML_pages_for_Mozilla.2FFirefox.2FInternet_Explorer.2FSafari.

Thanks for the reference, but I've already read it (many times in fact ;).

As far as I know, there's two way of configuring aspect ratio:

1. Using vlc.video.aspectRatio from javascript works BUT this setting
ends with the current playing media. It deactivates as soon as the
next item (in the playlist or in loop) starts. Also setting it is not
so easy, you need to wait for vlc.input.hasVout.

2. You could specify aspect-ratio as an option adding items to the
playlist using vlc.playlist.add(mrl,name,options). If you use always
the same aspect-ratio you need to add it as an option for all the
items.

Option 1 is difficult to set and has problems with playlists and loop
mode. Option 2 is more flexible but I feel it's over complicated just
to achieve a fixed aspect ratio. And finally BOTH options needs
JavaScript.

So I still think that a plain HTML (without JavaScript) solution is
possible, useful and simpler when we need just to put the correct
aspect ratio "globally". This solution could be just allowing a new
embed parameter.

Now I see that this is a very dangerous line:
            ppsz_argv[ppsz_argc++] = argv[i]; /*  DANGER! Just do it
under controlled environment. */
to put in a plugin exposed to the (wild, wild, wild) Internet.

It's allow the injection of all king of potential nasty options to the
starting of vlc. But I thought that we really don't need to allow any
value of aspect ratio. It could be very useful to just parse for the
more useful ones. I think of 4:3 and 16:9 ratios.

So perhaps it could be a way to close the worm can ;)

        else if( !strcmp( argn[i], "aspectratio" ) )
        {
            if( !strcmp( argv[i], "4:3" ) )
            {
                ppsz_argv[ppsz_argc++] = "--aspect-ratio=4:3";
            }
            if( !strcmp( argv[i], "16:9" ) )
            {
                ppsz_argv[ppsz_argc++] = "--aspect-ratio=16:9";
            }

I've discarded using switch to not evaluate argv[i].

Others ratios welcomed. And "--aspect-ratio=4:3" or "--aspect-ratio 4:3"?

Corrected patch attached.

Thanks for the comments :) keep them going!

Best regards
vicente

P.S. Obviously, source material as an incorrect aspect ratio
information, but another story.

>
> gtz,
> Jean-Paul Saman.
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Enable-aspectratio-parameter-in-the-mozilla-plugin.patch
Type: application/octet-stream
Size: 1213 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20090612/63cd1121/attachment.obj>