[PATCH] Minor bug(?) and modified behaviour for MP_CMD_TV/DVB_SET_CHANNEL
Hi, whilst looking through the code I noticed that if enabled, the MP_CMD_DVB_SET_CHANNEL falls through to the MP_CMD_TV_LAST_CHANNEL code. Also, whilst the MP_CMD_TV_STEP_CHANNEL code falls though (intentionally) to the equivalent code for DVB, this is not implemented with the set channel tv/dvb functionality. The following patch against CVS moves the break statement so that the MP_CMD_TV_SET_CHANNEL falls through to the dvb code, and the MP_CMD_DVB_SET_CHANNEL doesn't fall through to the MP_CMD_TV_LAST_CHANNEL code. Whilst the change does not change the behaviour of MP_CMD_DVB_SET_CHANNEL, since the MP_CMD_TV_LAST_CHANNEL code is not executed, I still thought it should be fixed. Also, I included the fall-through to DVB, because I _assumed_ (rightly or wrongly) that this what was intended when it was coded, as was done for MP_CMD_TV_STEP_CHANNEL explicitly. If you want them as two patches, I can do that, but it depends on whether you classify the new fall-through as a feature enhancement or as a bug fix, depending on the original code author's intent. ;-) //Andrew
calkin@ieee.org wrote:
Hi, whilst looking through the code I noticed that if enabled, the MP_CMD_DVB_SET_CHANNEL falls through to the MP_CMD_TV_LAST_CHANNEL code.
ok
Also, whilst the MP_CMD_TV_STEP_CHANNEL code falls though (intentionally) to the equivalent code for DVB, this is not implemented with the set channel tv/dvb functionality.
please, explain. Im not sure that I'm understanding correctly what you mean
The following patch against CVS moves the break statement so that the MP_CMD_TV_SET_CHANNEL falls through to the dvb code, and the MP_CMD_DVB_SET_CHANNEL doesn't fall through to the MP_CMD_TV_LAST_CHANNEL code.
the former is fine, but I don't understand the reason for the latter one. The commands are completely unrelated; only the keyboard mapping are the same for convenience
Whilst the change does not change the behaviour of MP_CMD_DVB_SET_CHANNEL, since the MP_CMD_TV_LAST_CHANNEL code is not executed, I still thought it should be fixed. Also, I included the fall-through to DVB, because I _assumed_ (rightly or wrongly) that this what was intended when it was coded, as was done for MP_CMD_TV_STEP_CHANNEL explicitly.
--- MPlayerCVS/mplayer.c 2006-02-25 15:09:54.000000000 +0800 +++ MPlayerCVS/mplayer.c 2006-03-02 03:06:22.000000000 +0800 @@ -3550,7 +3550,7 @@ //vo_osd_changed(OSDTYPE_SUBTITLE); } } - } break; + } /* fall through */ #ifdef HAS_DVBIN_SUPPORT case MP_CMD_DVB_SET_CHANNEL: { @@ -3574,6 +3574,7 @@ } } #endif /* HAS_DVBIN_SUPPORT */ +break; /* End MP_CMD_TV_SET_CHANNEL, MP_CMD_DVB_SET_CHANNEL */ case MP_CMD_TV_LAST_CHANNEL : { if (file_format == DEMUXER_TYPE_TV) { tv_last_channel((tvi_handle_t*)(demuxer->priv));
------------------------------------------------------------------------
I prefer the following: Index: mplayer.c =================================================================== RCS file: /cvsroot/mplayer/main/mplayer.c,v retrieving revision 1.910 diff -u -r1.910 mplayer.c --- mplayer.c 20 Feb 2006 09:36:09 -0000 1.910 +++ mplayer.c 2 Mar 2006 21:14:53 -0000 @@ -3590,6 +3590,7 @@ } } } + break; #endif /* HAS_DVBIN_SUPPORT */ case MP_CMD_TV_LAST_CHANNEL : { if (file_format == DEMUXER_TYPE_TV) { committed, thanks. Nico
participants (2)
-
calkin＠ieee.org -
Nico Sabbi