<div dir="ltr">Hi,<div><br></div><div>thx for the review. I think i fixed the mentioned issues: </div><div><br></div><div><div>From b4ec842c9fa813e5ea95aeb725151803c080f0c0 Mon Sep 17 00:00:00 2001</div><div>From: Matthias Schaff <<a href="mailto:mschaff@inovex.de">mschaff@inovex.de</a>></div><div>Date: Mon, 13 Jun 2016 08:26:36 +0200</div><div>Subject: [PATCH] addef FL32 Support to amem</div><div><br></div><div>---</div><div> modules/audio_output/amem.c | 40 ++++++++++++++++++++++++++++++----------</div><div> 1 file changed, 30 insertions(+), 10 deletions(-)</div><div><br></div><div>diff --git a/modules/audio_output/amem.c b/modules/audio_output/amem.c</div><div>index 9e9ece6..926a1ec 100644</div><div>--- a/modules/audio_output/amem.c</div><div>+++ b/modules/audio_output/amem.c</div><div>@@ -160,39 +160,60 @@ static void Stop (audio_output_t *aout)</div><div> static int Start (audio_output_t *aout, audio_sample_format_t *fmt)</div><div> {</div><div> aout_sys_t *sys = aout->sys;</div><div>- char format[5] = "S16N";</div><div> unsigned channels;</div><div>-</div><div>+ char * format;</div><div>+ format = var_InheritString(aout, "amem-format");</div><div>+ if(!format) {</div><div>+ return VLC_EGENERIC;</div><div>+ }</div><div>+ int i_format = 0;</div><div>+ if(!strcmp(format, "S16N"))</div><div>+ {</div><div>+ i_format = VLC_CODEC_S16N;</div><div>+ }</div><div>+ else if (!strcmp(format, "FL32"))</div><div>+ {</div><div>+ i_format = VLC_CODEC_FL32;</div><div>+ } else</div><div>+ {</div><div>+ msg_Warn(aout, "Audio format invalid: %s, amem only supports FL32 and S16N, using default: S16N", format);</div><div>+ i_format = VLC_CODEC_S16N;</div><div>+ strcpy(format, "S16N");</div><div>+ }</div><div> if (sys->setup != NULL)</div><div> {</div><div> channels = aout_FormatNbChannels(fmt);</div><div>-</div><div> sys->opaque = sys->setup_opaque;</div><div>- if (sys->setup (&sys->opaque, format, &fmt->i_rate, &channels))</div><div>+ if (sys->setup (&sys->opaque, format, &fmt->i_rate, &channels) != 0)</div><div>+ {</div><div>+ msg_Err(aout, "Audio format setup failure (audio playback skipped)");</div><div>+ free(format);</div><div> return VLC_EGENERIC;</div><div>+ }</div><div> }</div><div> else</div><div> {</div><div> fmt->i_rate = sys->rate;</div><div> channels = sys->channels;</div><div> }</div><div>+ fmt->i_format = i_format;</div><div> </div><div> /* Initialize volume (in case the UI changed volume before setup) */</div><div> sys->ready = true;</div><div> if (sys->set_volume != NULL)</div><div> sys->set_volume(sys->opaque, sys->volume, sys->mute);</div><div> </div><div>- /* Ensure that format is supported */</div><div>- if (fmt->i_rate == 0 || fmt->i_rate > 192000</div><div>- || channels == 0 || channels > AOUT_CHAN_MAX</div><div>- || strcmp(format, "S16N") /* TODO: amem-format */)</div><div>+ /* Ensure that format is supported, regarding rate and channels */</div><div>+ if (fmt->i_rate == 0 || fmt->i_rate > 192000 </div><div>+ || channels == 0 || channels > AOUT_CHAN_MAX)</div><div> {</div><div> msg_Err (aout, "format not supported: %s, %u channel(s), %u Hz",</div><div> format, channels, fmt->i_rate);</div><div> Stop (aout);</div><div>+ free(format);</div><div> return VLC_EGENERIC;</div><div> }</div><div>-</div><div>+ free(format);</div><div> /* channel mapping */</div><div> switch (channels)</div><div> {</div><div>@@ -227,7 +248,6 @@ static int Start (audio_output_t *aout, audio_sample_format_t *fmt)</div><div> vlc_assert_unreachable();</div><div> }</div><div> </div><div>- fmt->i_format = VLC_CODEC_S16N;</div><div> fmt->i_original_channels = fmt->i_physical_channels;</div><div> return VLC_SUCCESS;</div><div> }</div><div>-- </div><div>2.8.4</div><div><br></div></div><div><br></div><div>Best regards,<br></div><div>Matthias</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">2016-06-11 12:14 GMT+02:00 Tristan Matthews <span dir="ltr"><<a href="mailto:tmatth@videolan.org" target="_blank">tmatth@videolan.org</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<div><div class="h5"><br>
On Fri, Jun 10, 2016 at 1:39 PM, Matthias Schaff <<a href="mailto:mschaff@inovex.de">mschaff@inovex.de</a>> wrote:<br>
> From: Matthias Schaff <<a href="mailto:matthias.schaff@inovex.de">matthias.schaff@inovex.de</a>><br>
><br>
> ---<br>
> modules/audio_output/amem.c | 36 ++++++++++++++++++++++++++----------<br>
> 1 file changed, 26 insertions(+), 10 deletions(-)<br>
><br>
> diff --git a/modules/audio_output/amem.c b/modules/audio_output/amem.c<br>
> index 9e9ece6..fff761e 100644<br>
> --- a/modules/audio_output/amem.c<br>
> +++ b/modules/audio_output/amem.c<br>
> @@ -160,39 +160,56 @@ static void Stop (audio_output_t *aout)<br>
> static int Start (audio_output_t *aout, audio_sample_format_t *fmt)<br>
> {<br>
> aout_sys_t *sys = aout->sys;<br>
> - char format[5] = "S16N";<br>
> unsigned channels;<br>
> -<br>
> + char * format;<br>
> + format = var_InheritString(aout, "amem-format");<br>
> + int i_format = 0;<br>
> + if(!strcmp(format, "S16N"))<br>
> + {<br>
> + i_format = VLC_CODEC_S16N;<br>
> + }<br>
> + else if (!strcmp(format, "FL32"))<br>
> + {<br>
> + i_format = VLC_CODEC_FL32;<br>
> + } else<br>
> + {<br>
> + msg_Warn(aout, "Audio format invalid: %s, amem only supports FL32 and S16N, using default: S16N", format);<br>
> + i_format = VLC_CODEC_S16N;<br>
> + strcpy(format, "S16N");<br>
<br>
</div></div>Do you know for sure that the space allocated by var_InheritString()<br>
is big enough to hold this string?<br>
<span class=""><br>
> + }<br>
> if (sys->setup != NULL)<br>
> {<br>
> channels = aout_FormatNbChannels(fmt);<br>
> -<br>
> sys->opaque = sys->setup_opaque;<br>
> - if (sys->setup (&sys->opaque, format, &fmt->i_rate, &channels))<br>
> + if (sys->setup (&sys->opaque, format, &fmt->i_rate, &channels) != 0)<br>
> + {<br>
> + msg_Err(aout, "Audio format setup failure (audio playback skipped)");<br>
<br>
</span>You're leaking format here.<br>
<div><div class="h5"><br>
> return VLC_EGENERIC;<br>
> + }<br>
> }<br>
> else<br>
> {<br>
> fmt->i_rate = sys->rate;<br>
> channels = sys->channels;<br>
> }<br>
> + fmt->i_format = i_format;<br>
><br>
> /* Initialize volume (in case the UI changed volume before setup) */<br>
> sys->ready = true;<br>
> if (sys->set_volume != NULL)<br>
> sys->set_volume(sys->opaque, sys->volume, sys->mute);<br>
><br>
> - /* Ensure that format is supported */<br>
> - if (fmt->i_rate == 0 || fmt->i_rate > 192000<br>
> - || channels == 0 || channels > AOUT_CHAN_MAX<br>
> - || strcmp(format, "S16N") /* TODO: amem-format */)<br>
> + /* Ensure that format is supported, regarding rate and channels */<br>
> + if (fmt->i_rate == 0 || fmt->i_rate > 192000<br>
> + || channels == 0 || channels > AOUT_CHAN_MAX)<br>
> {<br>
> msg_Err (aout, "format not supported: %s, %u channel(s), %u Hz",<br>
> format, channels, fmt->i_rate);<br>
> Stop (aout);<br>
> + free(format);<br>
> return VLC_EGENERIC;<br>
> }<br>
> -<br>
> + free(format);<br>
> /* channel mapping */<br>
> switch (channels)<br>
> {<br>
> @@ -227,7 +244,6 @@ static int Start (audio_output_t *aout, audio_sample_format_t *fmt)<br>
> vlc_assert_unreachable();<br>
> }<br>
><br>
> - fmt->i_format = VLC_CODEC_S16N;<br>
> fmt->i_original_channels = fmt->i_physical_channels;<br>
> return VLC_SUCCESS;<br>
> }<br>
> --<br>
> 1.9.1<br>
><br>
> _______________________________________________<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/listinfo/vlc-devel</a><br>
<br>
</div></div>Best,<br>
Tristan<br>
<div class="HOEnZb"><div class="h5">_______________________________________________<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/listinfo/vlc-devel</a><br>
</div></div></blockquote></div><br></div>