Re: [FFmpeg-devel] [PATCHv4] ffplay: add support for interactive volume control

[Marton Balint]

On Tue, 29 Sep 2015, Ganesh Ajjanagadde wrote:


On Sun, Sep 27, 2015 at 5:09 PM, Marton Balint  wrote:


On Sun, 27 Sep 2015, Ganesh Ajjanagadde wrote:


This is a feature heavily inspired by the mpv player. At the moment,
methods
for adjusting volume in ffplay are rather clumsy: either one needs to set
it
system-wide, or one needs to set it via the volume filter.

This patch adds key bindings identical to the mpv defaults for
muting/unmuting
and increasing/decreasing the volume interactively without any
introduction of
external dependencies.

TODO: doc update, possible mouse button bindings (mpv has this).

Signed-off-by: Ganesh Ajjanagadde 
---
ffplay.c | 36 +++-
1 file changed, 35 insertions(+), 1 deletion(-)



Applied, thanks.


BTW, just a heads up: I won't be writing code for mouse wheel
bindings. The reason is that it is more annoying to support:
https://wiki.libsdl.org/MigrationGuide - basically old SDL 1.2 (which
is what many users use) treated it as a button, a mistake corrected in
2.0. I think the code will need to differ accordingly. Thus, configure
will need to export the SDL version, some ifdefry will need to be
done, and all that is gained is a feature which many users in fact
find annoying and explicitly disable:
https://forum.videolan.org/viewtopic.php?f=12=108470 (the very first
link in a search for "vlc mouse volume control" :D ).

However, I won't object to a patch as long as it handles both SDL
versions correctly and by default disables mouse volume control.


Actually I don't really like the idea of using the mouse wheel for volume 
control, so no hard feelings :).


By the way, I guess SDL2 compatiblity is more like a far goal, because it 
needs severe rework of the video part as well. (SDL_YUV_Overlay is gone) I 
have some patches I made earlier, I will publish it in a branch sometime.




Another side note on this business: VLC allows "boosting" volume to
125% (I recall even 400% in the past, not sure right now). I regard
this as a misfeature; even their devs later regretted it - maybe
that's why the 400 was toned down to 125. Note that ffplay still
supports such "boosting", but that needs to be done by a -af
volume=+xdB or something like that. I consider it a good thing: if
someone wants something extreme (which can clip the audio), they need
to explicitly enable it via a filter. Thus, I definitely did not
implement this and will object to a modification to ffplay trying to
do such a thing.


Agreed.



I will add a Changelog entry after a week, once bugs/issues with this
(if any) are handled.


Okay, i was wondering if it is big enough of a change for a changelog, but 
apparently it is, because you already got a phoronix article about it, so 
go ahead :)


Regards
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCHv4] ffplay: add support for interactive volume control

[Ganesh Ajjanagadde]

On Sun, Sep 27, 2015 at 5:09 PM, Marton Balint  wrote:
>
> On Sun, 27 Sep 2015, Ganesh Ajjanagadde wrote:
>
>> This is a feature heavily inspired by the mpv player. At the moment,
>> methods
>> for adjusting volume in ffplay are rather clumsy: either one needs to set
>> it
>> system-wide, or one needs to set it via the volume filter.
>>
>> This patch adds key bindings identical to the mpv defaults for
>> muting/unmuting
>> and increasing/decreasing the volume interactively without any
>> introduction of
>> external dependencies.
>>
>> TODO: doc update, possible mouse button bindings (mpv has this).
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>> ffplay.c | 36 +++-
>> 1 file changed, 35 insertions(+), 1 deletion(-)
>>
>
> Applied, thanks.

BTW, just a heads up: I won't be writing code for mouse wheel
bindings. The reason is that it is more annoying to support:
https://wiki.libsdl.org/MigrationGuide - basically old SDL 1.2 (which
is what many users use) treated it as a button, a mistake corrected in
2.0. I think the code will need to differ accordingly. Thus, configure
will need to export the SDL version, some ifdefry will need to be
done, and all that is gained is a feature which many users in fact
find annoying and explicitly disable:
https://forum.videolan.org/viewtopic.php?f=12=108470 (the very first
link in a search for "vlc mouse volume control" :D ).

However, I won't object to a patch as long as it handles both SDL
versions correctly and by default disables mouse volume control.

Another side note on this business: VLC allows "boosting" volume to
125% (I recall even 400% in the past, not sure right now). I regard
this as a misfeature; even their devs later regretted it - maybe
that's why the 400 was toned down to 125. Note that ffplay still
supports such "boosting", but that needs to be done by a -af
volume=+xdB or something like that. I consider it a good thing: if
someone wants something extreme (which can clip the audio), they need
to explicitly enable it via a filter. Thus, I definitely did not
implement this and will object to a modification to ffplay trying to
do such a thing.

I will add a Changelog entry after a week, once bugs/issues with this
(if any) are handled.

>
> Marton
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCHv4] ffplay: add support for interactive volume control

[Ganesh Ajjanagadde]

On Sun, Sep 27, 2015 at 12:06 PM, Stefano Sabatini  wrote:
> On date Sunday 2015-09-27 10:41:37 -0400, Ganesh Ajjanagadde encoded:
>> This is a feature heavily inspired by the mpv player. At the moment, methods
>> for adjusting volume in ffplay are rather clumsy: either one needs to set it
>> system-wide, or one needs to set it via the volume filter.
>>
>> This patch adds key bindings identical to the mpv defaults for 
>> muting/unmuting
>> and increasing/decreasing the volume interactively without any introduction 
>> of
>> external dependencies.
>>
>
>> TODO: doc update, possible mouse button bindings (mpv has this).
>
> Also update show_help_default() output while at it.

So I am adding it to the doc patch - I consider it ambiguous where it
needs to go - logically it is related to documentation, but it belongs
in the same file as ffplay. I personally like the "logical" split
more, so I have placed it in the doc patch.

>
> [...]
> --
> FFmpeg = Forgiving & Free Multipurpose Philosofic Elaborated Goblin
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCHv4] ffplay: add support for interactive volume control

[Ganesh Ajjanagadde]

This is a feature heavily inspired by the mpv player. At the moment, methods
for adjusting volume in ffplay are rather clumsy: either one needs to set it
system-wide, or one needs to set it via the volume filter.

This patch adds key bindings identical to the mpv defaults for muting/unmuting
and increasing/decreasing the volume interactively without any introduction of
external dependencies.

TODO: doc update, possible mouse button bindings (mpv has this).

Signed-off-by: Ganesh Ajjanagadde 
---
 ffplay.c | 36 +++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/ffplay.c b/ffplay.c
index 2bb5d26..21600d0 100644
--- a/ffplay.c
+++ b/ffplay.c
@@ -73,6 +73,9 @@ const int program_birth_year = 2003;
 /* Calculate actual buffer size keeping in mind not cause too frequent audio 
callbacks */
 #define SDL_AUDIO_MAX_CALLBACKS_PER_SEC 30
 
+/* Step size for volume control */
+#define SDL_VOLUME_STEP (SDL_MIX_MAXVOLUME / 50)
+
 /* no AV sync correction is done if below the minimum AV sync threshold */
 #define AV_SYNC_THRESHOLD_MIN 0.04
 /* AV sync correction is done if above the maximum AV sync threshold */
@@ -246,6 +249,8 @@ typedef struct VideoState {
 unsigned int audio_buf1_size;
 int audio_buf_index; /* in bytes */
 int audio_write_buf_size;
+int audio_volume;
+int muted;
 struct AudioParams audio_src;
 #if CONFIG_AVFILTER
 struct AudioParams audio_filter_src;
@@ -1349,6 +1354,16 @@ static void toggle_pause(VideoState *is)
 is->step = 0;
 }
 
+static void toggle_mute(VideoState *is)
+{
+is->muted = !is->muted;
+}
+
+static void update_volume(VideoState *is, int sign, int step)
+{
+is->audio_volume = av_clip(is->audio_volume + sign * step, 0, 
SDL_MIX_MAXVOLUME);
+}
+
 static void step_to_next_frame(VideoState *is)
 {
 /* if the stream is paused unpause it, then step */
@@ -2448,7 +2463,13 @@ static void sdl_audio_callback(void *opaque, Uint8 
*stream, int len)
 len1 = is->audio_buf_size - is->audio_buf_index;
 if (len1 > len)
 len1 = len;
-memcpy(stream, (uint8_t *)is->audio_buf + is->audio_buf_index, len1);
+if (!is->muted && is->audio_volume == SDL_MIX_MAXVOLUME)
+memcpy(stream, (uint8_t *)is->audio_buf + is->audio_buf_index, 
len1);
+else {
+memset(stream, is->silence_buf[0], len1);
+if (!is->muted)
+SDL_MixAudio(stream, (uint8_t *)is->audio_buf + 
is->audio_buf_index, len1, is->audio_volume);
+}
 len -= len1;
 stream += len1;
 is->audio_buf_index += len1;
@@ -3125,6 +3146,8 @@ static VideoState *stream_open(const char *filename, 
AVInputFormat *iformat)
 init_clock(>audclk, >audioq.serial);
 init_clock(>extclk, >extclk.serial);
 is->audio_clock_serial = -1;
+is->audio_volume = SDL_MIX_MAXVOLUME;
+is->muted = 0;
 is->av_sync_type = av_sync_type;
 is->read_tid = SDL_CreateThread(read_thread, is);
 if (!is->read_tid) {
@@ -3314,6 +3337,17 @@ static void event_loop(VideoState *cur_stream)
 case SDLK_SPACE:
 toggle_pause(cur_stream);
 break;
+case SDLK_m:
+toggle_mute(cur_stream);
+break;
+case SDLK_KP_MULTIPLY:
+case SDLK_0:
+update_volume(cur_stream, 1, SDL_VOLUME_STEP);
+break;
+case SDLK_KP_DIVIDE:
+case SDLK_9:
+update_volume(cur_stream, -1, SDL_VOLUME_STEP);
+break;
 case SDLK_s: // S: Step to next frame
 step_to_next_frame(cur_stream);
 break;
-- 
2.5.3

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCHv4] ffplay: add support for interactive volume control

[Marton Balint]

On Sun, 27 Sep 2015, Ganesh Ajjanagadde wrote:


This is a feature heavily inspired by the mpv player. At the moment, methods
for adjusting volume in ffplay are rather clumsy: either one needs to set it
system-wide, or one needs to set it via the volume filter.

This patch adds key bindings identical to the mpv defaults for muting/unmuting
and increasing/decreasing the volume interactively without any introduction of
external dependencies.

TODO: doc update, possible mouse button bindings (mpv has this).

Signed-off-by: Ganesh Ajjanagadde 
---
ffplay.c | 36 +++-
1 file changed, 35 insertions(+), 1 deletion(-)



Applied, thanks.

Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel