<div dir="auto">Hi, Filip<div dir="auto"><br></div><div dir="auto">Thank you. I hope that my work will be helpful here at VLC. Regarding the bugs I chose a few that I thought would be entry-level and I will start working at one of them this weekend. </div><div dir="auto"><br></div><div dir="auto">Best regards,</div><div dir="auto">Andrei Ciurea</div></div><div class="gmail_extra"><br><div class="gmail_quote">On 24 Feb 2017 9:11 pm, "Filip Roséen" <<a href="mailto:filip@atch.se">filip@atch.se</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><u></u>
<div>
<p>Hi Andrei,</p>
<p>On 2017-02-24 18:07, Andrei Ciurea wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> Thank you for review. The comments were for me to be sure that i remember
what i've changed (at a search in the whole package). I'll take good care
next time and i will make the changes and send this patch again.</code></pre>
</blockquote>
<p>Best of luck!</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> I intend to start working at another bug, do you have any advice for me?</code></pre>
</blockquote>
<p>I have been meaning to compile an “entry-level list of bugs” for so so long, but I have not gotten around to it; with that in mind I sadly do not have any particular bug to recommend.</p>
<p>Given that it seems that you have a lot of interest in contributing to VLC, I’d like to take the opportunity to welcome you onto <code>vlc-devel</code> (and <code>#videolan</code> @ <code>freenode</code>).</p>
<p>All the best,<br>
Filip</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> Best Regards,
Andrei Ciurea
2017-02-24 14:38 GMT+02:00 Filip Roséen <<a href="mailto:filip@atch.se" target="_blank">filip@atch.se</a>>:</code></pre>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> Hi Andrei,
I am glad you figured out how to create a patch and sent it to vlc-devel,
I was about to ping you on IRC a few hours ago to see how you were doing
(but I forgot).
In order increase the chance of merging the changes some slight clean-up
(mostly regarding comments) is recommended (see the rest of this extremely
minor review).
I am aware that you simply copied the implementation related to cycling
audio-tracks, so my comments regarding style and such are just for future
reference (especially if someone decides to refactor the implementation as
mentioned).
On 2017-02-24 13:59, Andrei Ciurea wrote:
---
include/vlc_keys.h | 3 +++
modules/control/hotkeys.c | 37 ++++++++++++++++++++++++++++++<wbr>+++++++
src/config/keys.c | 4 ++++
src/libvlc-module.c | 7 +++++++
4 files changed, 51 insertions(+)
Please revise the commit message so that it includes a description, and a
more suitable title.
diff --git a/include/vlc_keys.h b/include/vlc_keys.h
index 8d8df53..bb1f18e 100644
--- a/include/vlc_keys.h
+++ b/include/vlc_keys.h
@@ -100,6 +100,8 @@
#define KEY_ZOOM_OUT 0x00610000
#define KEY_BRIGHTNESS_UP 0x00620000
#define KEY_BRIGHTNESS_DOWN 0x00630000
+ //my mod. asignin' sa key for video_track
I assume the above comment to be an unfortunate leftover, but please
remove such from the final patch as they have no value inside the codebase.
+#define KEY_MEDIA_VIDEO 0x00640000
#define KEY_MOUSEWHEELUP 0x00F00000
#define KEY_MOUSEWHEELDOWN 0x00F10000
@@ -171,6 +173,7 @@ typedef enum vlc_action {
ACTIONID_SUBPOS_UP,
ACTIONID_SUBPOS_DOWN,
ACTIONID_AUDIO_TRACK,
+ ACTIONID_VIDEO_TRACK,
ACTIONID_SUBTITLE_TRACK,
ACTIONID_SUBTITLE_TOGGLE,
ACTIONID_SUBTITLE_TEXT_SCALE_<wbr>NORMAL,
diff --git a/modules/control/hotkeys.c b/modules/control/hotkeys.c
index 4e4f368..3338a1f 100644
--- a/modules/control/hotkeys.c
+++ b/modules/control/hotkeys.c
@@ -737,6 +737,43 @@ static int PutAction( intf_thread_t *p_intf, input_thread_t *p_input,
var_FreeList( &list, &list2 );
}
break;
+ //my mod
See remark about leftover comments.
+ case ACTIONID_VIDEO_TRACK:
+ if( p_input )
+ {
+ vlc_value_t val, list, list2;
+ int i_count, i;
There is no need to seperate declaration and initialization ( i_count,
and i) like in the above. Just do declaration and initialization in one
step from the moment you need the variable (increases readability and
heavily simplifies refactoring).
+ var_Get( p_input, "video-es", &val );
+ var_Change( p_input, "video-es", VLC_VAR_GETCHOICES,
+ &list, &list2 );
+ i_count = list.p_list->i_count;
+ if( i_count > 1 )
+ {
To increase readability you could use list.p_list->i_count in the
*if-condition*, and then declare a variable within the *if-body* for
easier access (making it clear that i_count is only used in this scope).
+ for( i = 0; i < i_count; i++ )
+ {
+ if( val.i_int == list.p_list->p_values[i].i_int )
+ {
+ break;
+ }
+ }
+ /* value of video-es was not in choices list */
+ if( i == i_count )
+ {
+ msg_Warn( p_input,
+ "invalid current video track, selecting 0" );
+ i = 0;
+ }
+ else if( i == i_count - 1 )
+ i = 1;
+ else
+ i++;
+ var_Set( p_input, "video-es", list.p_list->p_values[i] );
+ DisplayMessage( p_vout, _("Video track: %s"),
+ list2.p_list->p_values[i].psz_<wbr>string );
+ }
+ var_FreeList( &list, &list2 );
+ }
+ break;
The implementation under case ACTIONID_VIDEO_TRACK only differs from the
already present case ACTIONID_AUDIO_TRACK in terms of what variable is
being accessed, and the strings used for diagnostics.
Have you considered refactoring so that both cases uses the same
implementation?
case ACTIONID_SUBTITLE_TRACK:
if( p_input )
{
diff --git a/src/config/keys.c b/src/config/keys.c
index 336466b..3d28b26 100644
--- a/src/config/keys.c
+++ b/src/config/keys.c
@@ -86,6 +86,8 @@ static const struct key_descriptor_s vlc_keys[] =
{ N_("Left"), KEY_LEFT },
{ N_("Media Angle"), KEY_MEDIA_ANGLE },
{ N_("Media Audio Track"), KEY_MEDIA_AUDIO },
+ //my mod asign key descriptor and vlc_key
See remark about leftover comments.
+ { N_("Media Video Track"), KEY_MEDIA_VIDEO },
{ N_("Media Forward"), KEY_MEDIA_FORWARD },
{ N_("Media Menu"), KEY_MEDIA_MENU },
{ N_("Media Next Frame"), KEY_MEDIA_FRAME_NEXT },
@@ -273,6 +275,8 @@ static const struct action actions[] =
/* *MUST* be sorted (ASCII order) */
{ "aspect-ratio", ACTIONID_ASPECT_RATIO, },
{ "audio-track", ACTIONID_AUDIO_TRACK, },
+ //my mod. asigning a actionid to video track
See remark about leftover comments.
+ { "video-track", ACTIONID_VIDEO_TRACK, },
{ "audiodelay-down", ACTIONID_AUDIODELAY_DOWN, },
{ "audiodelay-up", ACTIONID_AUDIODELAY_UP, },
{ "audiodevice-cycle", ACTIONID_AUDIODEVICE_CYCLE, },
diff --git a/src/libvlc-module.c b/src/libvlc-module.c
index 6d60e80..b2e6988 100644
--- a/src/libvlc-module.c
+++ b/src/libvlc-module.c
@@ -1357,6 +1357,8 @@ static const char *const mouse_wheel_texts[] = {
#define AUDIO_TRACK_KEY_TEXT N_("Cycle audio track")
#define AUDIO_TRACK_KEY_LONGTEXT N_("Cycle through the available audio tracks(languages).")
+#define VIDEO_TRACK_KEY_TEXT N_("Cycle video track")
+#define VIDEO_TRACK_KEY_LONGTEXT N_("Cycle through the available video tracks.")
#define SUBTITLE_TRACK_KEY_TEXT N_("Cycle subtitle track")
#define SUBTITLE_TRACK_KEY_LONGTEXT N_("Cycle through the available subtitle tracks.")
#define SUBTITLE_TOGGLE_KEY_TEXT N_("Toggle subtitles")
@@ -2198,6 +2200,7 @@ vlc_module_begin ()
# define KEY_AUDIODELAY_UP "g"
# define KEY_AUDIODELAY_DOWN "f"
# define KEY_AUDIO_TRACK "l"
+# define KEY_VIDEO_TRACK "/"
# define KEY_SUBTITLE_TRACK "s"
# define KEY_SUBTITLE_TOGGLE "Shift+s"
# define KEY_PROGRAM_SID_NEXT "x"
@@ -2340,6 +2343,7 @@ vlc_module_begin ()
# define KEY_LOOP "l"
# define KEY_AUDIO_TRACK "b"
+# define KEY_VIDEO_TRACK "/"
# define KEY_SUBTITLE_TRACK "v"
# define KEY_SUBTITLE_TOGGLE "Shift+v"
# define KEY_PROGRAM_SID_NEXT "x"
@@ -2512,6 +2516,9 @@ vlc_module_begin ()
AUDIODELAY_DOWN_KEY_TEXT, AUDIODELAY_DOWN_KEY_LONGTEXT, true )
add_key( "key-audio-track", KEY_AUDIO_TRACK, AUDIO_TRACK_KEY_TEXT,
AUDIO_TRACK_KEY_LONGTEXT, false )
+ //my mod
See remark about leftover comments.
+ add_key( "key-video-track", KEY_VIDEO_TRACK, VIDEO_TRACK_KEY_TEXT,
+ VIDEO_TRACK_KEY_LONGTEXT, false )
add_key( "key-audiodevice-cycle", KEY_AUDIODEVICE_CYCLE, AUDI_DEVICE_CYCLE_KEY_TEXT,
AUDI_DEVICE_CYCLE_KEY_<wbr>LONGTEXT, false )
add_key( "key-subtitle-track", KEY_SUBTITLE_TRACK,
Best Regards,
Filip Roséen
______________________________<wbr>_________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
<a href="https://mailman.videolan.org/listinfo/vlc-devel" target="_blank">https://mailman.videolan.org/<wbr>listinfo/vlc-devel</a></code></pre>
</blockquote>
</blockquote>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> ______________________________<wbr>_________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
<a href="https://mailman.videolan.org/listinfo/vlc-devel" target="_blank">https://mailman.videolan.org/<wbr>listinfo/vlc-devel</a></code></pre>
</blockquote>
</div>
<br>______________________________<wbr>_________________<br>
vlc-devel mailing list<br>
To unsubscribe or modify your subscription options:<br>
<a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/<wbr>listinfo/vlc-devel</a><br></blockquote></div></div>