Reimar: Thanks for your reply. I am using mencoder as part of a mediatomb application that recodes video for tablet devices, so mencoder is still a useful thing. Sorry to hear it isn't being maintained, but that's the way it goes I guess. I need to do the "if" and update dvdsub_id because I am following the pattern used farther down in the source file. Also, the code that uses the value won't do the right thing unless I set it. In fact, when testing the application, I noticed that if I didn't set it, I didn't fix the bug. Regards, Ed Batutis On Thu, Sep 4, 2014 at 2:41 PM, Reimar Döffinger <Reimar.Doeffinger@gmx.de> wrote:

On Tue, Mar 11, 2014 at 01:12:28PM -0400, Edward Batutis wrote:

...

Hi,

This is my very first patch for mplayer/mencoder.

And promptly your patch fell through the cracks :( Very sorry about that! Note that mencoder isn't properly maintained anymore, which should explain why things don't work in it that work in MPlayer, but also is the reason for my main comment:

...

+ if(dvdsub_lang && dvdsub_id==-1) vobsub_set_from_lang(vo_vobsub,dvdsub_lang,&dvdsub_id); This is quite a bit different from what MPlayer does, increasing the risk it will break in the future (I suspect it also means they will behave different when they are given a certain set of options).

Why can't you just do vobsub_set_from_lang(vo_vobsub,dvdsub_lang); here like MPlayer does (without the if and without changing dvdsub_id)?

Thanks, Reimar

I'm sure what you say is true. Without this patch I believe that subtitle selection is completely broken in mencoder - I tried a lot of things before making this patch. The patch has functional limitations, but it gives a user at least one way to select a subtitle. The coding style is not what I would personally prefer, but it matches the style of the existing code to the best of my ability. Regards, =Ed On Sat, Sep 6, 2014 at 4:06 PM, Reimar Döffinger <Reimar.Doeffinger@gmx.de> wrote:

...

Reimar:

Thanks for your reply.

I am using mencoder as part of a mediatomb application that recodes video for tablet devices, so mencoder is still a useful thing. Sorry to hear it isn't being maintained, but that's the way it goes I guess.

I need to do the "if" and update dvdsub_id because I am following the pattern used farther down in the source file. Also, the code that uses

On Fri, Sep 05, 2014 at 06:30:26PM -0400, Edward Batutis wrote: the

...

value won't do the right thing unless I set it.

In fact, when testing the application, I noticed that if I didn't set it, I didn't fix the bug.

This seems to be due to the fact the mencoder misses MPlayer's select_subtitle function, that probably should be shared and reused in order to get consistent behaviour. In addition mencoder misses MPlayer's load_vob_subtitle and surrounding code, which leads to other inconsistencies like no support for only forced subtitles.