[PATCH] Report the reason to exit for -identify
Frontends may need to know the reason for what mplayer has exited (quitted by the user, end of file...). It already prints that info with these messages: Exiting... (Quit) Exiting... (End of file) The problem is that those strings are translatable strings. So if mplayer is compiled with another language as default, it will print other strings, making things harder for frontends. To fix the problem I suggest that mplayer would also print some strings for -identify, for example: ID_EXIT=QUIT ID_EXIT=EOF ID_EXIT=ERROR I attach a patch which does that. Although probably it's not implemented in the right way... What do you think? -- rvm
El Lunes, 3 de Noviembre de 2008 00:35, rvm escribió:
Frontends may need to know the reason for what mplayer has exited (quitted by the user, end of file...). It already prints that info with these messages:
Exiting... (Quit) Exiting... (End of file)
The problem is that those strings are translatable strings. So if mplayer is compiled with another language as default, it will print other strings, making things harder for frontends.
To fix the problem I suggest that mplayer would also print some strings for -identify, for example:
ID_EXIT=QUIT ID_EXIT=EOF ID_EXIT=ERROR
I attach a patch which does that. Although probably it's not implemented in the right way...
No comments? -- rvm
Hi, On Monday 10 November 2008 03:08, rvm wrote:
El Lunes, 3 de Noviembre de 2008 00:35, rvm escribió:
Frontends may need to know the reason for what mplayer has exited (quitted by the user, end of file...). It already prints that info with these messages:
Exiting... (Quit) Exiting... (End of file)
The problem is that those strings are translatable strings. So if mplayer is compiled with another language as default, it will print other strings, making things harder for frontends.
To fix the problem I suggest that mplayer would also print some strings for -identify, for example:
ID_EXIT=QUIT ID_EXIT=EOF ID_EXIT=ERROR
I attach a patch which does that. Although probably it's not implemented in the right way...
No comments?
It looks OK, but I'm not feeling myself brave enough to apply... P.S. Well, maybe it is better to use switch() block instead of lot of 'if' statements? -- Best regards, Andrew
El Lunes, 10 de Noviembre de 2008 01:29, Andrew Savchenko escribió:
Hi,
On Monday 10 November 2008 03:08, rvm wrote:
El Lunes, 3 de Noviembre de 2008 00:35, rvm escribió:
Frontends may need to know the reason for what mplayer has exited (quitted by the user, end of file...). It already prints that info with these messages:
Exiting... (Quit) Exiting... (End of file)
The problem is that those strings are translatable strings. So if mplayer is compiled with another language as default, it will print other strings, making things harder for frontends.
To fix the problem I suggest that mplayer would also print some strings for -identify, for example:
ID_EXIT=QUIT ID_EXIT=EOF ID_EXIT=ERROR
I attach a patch which does that. Although probably it's not implemented in the right way...
No comments?
It looks OK, but I'm not feeling myself brave enough to apply...
P.S. Well, maybe it is better to use switch() block instead of lot of 'if' statements?
I think I can't use a switch. "how", "MSGTR_Exit_quit" and so on are pointers (const char *), not integers. Compiling fails if I try to use switch with them. Maybe I should have used strcmp instead so this would be clearer. I send a version of the patch with it. Maybe the best would be to change the type of "how" from const char * to an int or enum, and then print one thing or another (in a switch) instead of having to compare the strings, but that would require some deeper changes. -- rvm
Hello, On Mon, Nov 10, 2008 at 11:50:53PM +0100, rvm wrote:
Maybe the best would be to change the type of "how" from const char * to an int or enum, and then print one thing or another (in a switch) instead of having to compare the strings, but that would require some deeper changes.
IMO that is the only acceptable solution. Also I think you should always print an ID_EXIT even when you do not have any "how" set. To distinguish the error conditions you could also just use different exit codes. Greetings, Reimar Döffinger
El Martes, 11 de Noviembre de 2008 08:33, Reimar Döffinger escribió:
Hello,
On Mon, Nov 10, 2008 at 11:50:53PM +0100, rvm wrote:
Maybe the best would be to change the type of "how" from const char * to an int or enum, and then print one thing or another (in a switch) instead of having to compare the strings, but that would require some deeper changes.
IMO that is the only acceptable solution. Also I think you should always print an ID_EXIT even when you do not have any "how" set. To distinguish the error conditions you could also just use different exit codes.
Attached is my first try to make the patch using an enum for the "how" parameter. -- rvm
Hi, On Wednesday 12 November 2008 02:36, rvm wrote: [...]
Index: mplayer.c ================================================================ === --- mplayer.c (revisión: 27904) +++ mplayer.c (copia de trabajo) @@ -685,7 +685,7 @@ current_module=NULL; }
-void exit_player_with_rc(const char* how, int rc){ +void exit_player_with_rc(ExitReason how, int rc){
IMHO, exit_reason_t will suit better for mplayer.c coding style. [...]
switch(how) { + case EXIT_QUIT: + mp_msg(MSGT_CPLAYER,MSGL_INFO,MSGTR_ExitingHow,MSGTR_Exit_quit); + mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_EXIT=QUIT\n"); + break; + case EXIT_EOF: + mp_msg(MSGT_CPLAYER,MSGL_INFO,MSGTR_ExitingHow,MSGTR_Exit_eof); + mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_EXIT=EOF\n"); + break; + case EXIT_ERROR: + mp_msg(MSGT_CPLAYER,MSGL_INFO,MSGTR_ExitingHow,MSGTR_Exit_error) ; + mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_EXIT=ERROR\n"); + break; + default: + mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_EXIT\n");
Maybe you should use "ID_EXIT=NULL" (or NONE if you like it more) for consistency? Otherwise it looks ok for me. -- Best regards, Andrew
El Sábado, 15 de Noviembre de 2008 16:09, Andrew Savchenko escribió:
Hi,
On Wednesday 12 November 2008 02:36, rvm wrote: [...]
Index: mplayer.c ================================================================ === --- mplayer.c (revisión: 27904) +++ mplayer.c (copia de trabajo) @@ -685,7 +685,7 @@ current_module=NULL; }
-void exit_player_with_rc(const char* how, int rc){ +void exit_player_with_rc(ExitReason how, int rc){
IMHO, exit_reason_t will suit better for mplayer.c coding style.
[...]
switch(how) { + case EXIT_QUIT: + mp_msg(MSGT_CPLAYER,MSGL_INFO,MSGTR_ExitingHow,MSGTR_Exit_quit); + mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_EXIT=QUIT\n"); + break; + case EXIT_EOF: + mp_msg(MSGT_CPLAYER,MSGL_INFO,MSGTR_ExitingHow,MSGTR_Exit_eof); + mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_EXIT=EOF\n"); + break; + case EXIT_ERROR: + mp_msg(MSGT_CPLAYER,MSGL_INFO,MSGTR_ExitingHow,MSGTR_Exit_error) ; + mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_EXIT=ERROR\n"); + break; + default: + mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_EXIT\n");
Maybe you should use "ID_EXIT=NULL" (or NONE if you like it more) for consistency?
Otherwise it looks ok for me.
Attached patch with the suggested changes. -- rvm
El Domingo, 16 de Noviembre de 2008 00:06, rvm escribió:
El Sábado, 15 de Noviembre de 2008 16:09, Andrew Savchenko escribió:
Hi,
On Wednesday 12 November 2008 02:36, rvm wrote: [...]
Index: mplayer.c ================================================================ === --- mplayer.c (revisión: 27904) +++ mplayer.c (copia de trabajo) @@ -685,7 +685,7 @@ current_module=NULL; }
-void exit_player_with_rc(const char* how, int rc){ +void exit_player_with_rc(ExitReason how, int rc){
IMHO, exit_reason_t will suit better for mplayer.c coding style.
[...]
switch(how) { + case EXIT_QUIT: + mp_msg(MSGT_CPLAYER,MSGL_INFO,MSGTR_ExitingHow,MSGTR_Exit_quit); + mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_EXIT=QUIT\n"); + break; + case EXIT_EOF: + mp_msg(MSGT_CPLAYER,MSGL_INFO,MSGTR_ExitingHow,MSGTR_Exit_eof); + mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_EXIT=EOF\n"); + break; + case EXIT_ERROR: + mp_msg(MSGT_CPLAYER,MSGL_INFO,MSGTR_ExitingHow,MSGTR_Exit_error) ; + mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_EXIT=ERROR\n"); + break; + default: + mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_EXIT\n");
Maybe you should use "ID_EXIT=NULL" (or NONE if you like it more) for consistency?
Otherwise it looks ok for me.
Attached patch with the suggested changes.
Attached new version of the patch for r28011 (previous one doesn't work anymore). Is there anything else which needs to be fixed in the patch? -- rvm
On Mon, Nov 24, 2008 at 02:47:20AM +0100, rvm wrote:
Attached new version of the patch for r28011 (previous one doesn't work anymore).
Is there anything else which needs to be fixed in the patch?
Well, in the long term a table instead of the switch might be better, but IMO the patch is ok as it is. Greetings, Reimar Döffinger
El Lunes, 24 de Noviembre de 2008 10:51, Reimar Döffinger escribió:
On Mon, Nov 24, 2008 at 02:47:20AM +0100, rvm wrote:
Attached new version of the patch for r28011 (previous one doesn't work anymore).
Is there anything else which needs to be fixed in the patch?
Well, in the long term a table instead of the switch might be better, but IMO the patch is ok as it is.
So can it be committed then? -- rvm
On Sun, Nov 30, 2008 at 12:20:41AM +0100, rvm wrote:
El Lunes, 24 de Noviembre de 2008 10:51, Reimar Döffinger escribió:
On Mon, Nov 24, 2008 at 02:47:20AM +0100, rvm wrote:
Attached new version of the patch for r28011 (previous one doesn't work anymore).
Is there anything else which needs to be fixed in the patch?
Well, in the long term a table instead of the switch might be better, but IMO the patch is ok as it is.
So can it be committed then?
Applied by Reimar. Diego
participants (5)
-
Andrew Savchenko -
Andrew Savchenko -
Diego Biurrun -
Reimar Döffinger -
rvm