[vlc-devel] [PATCH] amem: Added FL32 support to amem

[Matthias Schaff]

Hi,

thx for the review. I think i fixed the mentioned issues:

>From b4ec842c9fa813e5ea95aeb725151803c080f0c0 Mon Sep 17 00:00:00 2001
From: Matthias Schaff <mschaff at inovex.de>
Date: Mon, 13 Jun 2016 08:26:36 +0200
Subject: [PATCH] addef FL32 Support to amem

---
 modules/audio_output/amem.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/modules/audio_output/amem.c b/modules/audio_output/amem.c
index 9e9ece6..926a1ec 100644
--- a/modules/audio_output/amem.c
+++ b/modules/audio_output/amem.c
@@ -160,39 +160,60 @@ static void Stop (audio_output_t *aout)
 static int Start (audio_output_t *aout, audio_sample_format_t *fmt)
 {
     aout_sys_t *sys = aout->sys;
-    char format[5] = "S16N";
     unsigned channels;
-
+    char * format;
+    format = var_InheritString(aout, "amem-format");
+    if(!format) {
+      return VLC_EGENERIC;
+    }
+    int i_format = 0;
+    if(!strcmp(format, "S16N"))
+    {
+        i_format = VLC_CODEC_S16N;
+    }
+    else if (!strcmp(format, "FL32"))
+    {
+        i_format = VLC_CODEC_FL32;
+    } else
+    {
+        msg_Warn(aout, "Audio format invalid: %s, amem only supports FL32
and S16N, using default: S16N", format);
+        i_format = VLC_CODEC_S16N;
+        strcpy(format, "S16N");
+    }
     if (sys->setup != NULL)
     {
         channels = aout_FormatNbChannels(fmt);
-
         sys->opaque = sys->setup_opaque;
-        if (sys->setup (&sys->opaque, format, &fmt->i_rate, &channels))
+        if (sys->setup (&sys->opaque, format, &fmt->i_rate, &channels) !=
0)
+        {
+            msg_Err(aout, "Audio format setup failure (audio playback
skipped)");
+            free(format);
             return VLC_EGENERIC;
+        }
     }
     else
     {
         fmt->i_rate = sys->rate;
         channels = sys->channels;
     }
+    fmt->i_format = i_format;

     /* Initialize volume (in case the UI changed volume before setup) */
     sys->ready = true;
     if (sys->set_volume != NULL)
         sys->set_volume(sys->opaque, sys->volume, sys->mute);

-    /* Ensure that format is supported */
-    if (fmt->i_rate == 0 || fmt->i_rate > 192000
-     || channels == 0 || channels > AOUT_CHAN_MAX
-     || strcmp(format, "S16N") /* TODO: amem-format */)
+    /* Ensure that format is supported, regarding rate and channels */
+    if (fmt->i_rate == 0 || fmt->i_rate > 192000
+     || channels == 0 || channels > AOUT_CHAN_MAX)
     {
         msg_Err (aout, "format not supported: %s, %u channel(s), %u Hz",
                  format, channels, fmt->i_rate);
         Stop (aout);
+        free(format);
         return VLC_EGENERIC;
     }
-
+    free(format);
     /* channel mapping */
     switch (channels)
     {
@@ -227,7 +248,6 @@ static int Start (audio_output_t *aout,
audio_sample_format_t *fmt)
             vlc_assert_unreachable();
     }

-    fmt->i_format = VLC_CODEC_S16N;
     fmt->i_original_channels = fmt->i_physical_channels;
     return VLC_SUCCESS;
 }
-- 
2.8.4


Best regards,
Matthias


2016-06-11 12:14 GMT+02:00 Tristan Matthews <tmatth at videolan.org>:

> Hi,
>
> On Fri, Jun 10, 2016 at 1:39 PM, Matthias Schaff <mschaff at inovex.de>
> wrote:
> > From: Matthias Schaff <matthias.schaff at inovex.de>
> >
> > ---
> >  modules/audio_output/amem.c | 36 ++++++++++++++++++++++++++----------
> >  1 file changed, 26 insertions(+), 10 deletions(-)
> >
> > diff --git a/modules/audio_output/amem.c b/modules/audio_output/amem.c
> > index 9e9ece6..fff761e 100644
> > --- a/modules/audio_output/amem.c
> > +++ b/modules/audio_output/amem.c
> > @@ -160,39 +160,56 @@ static void Stop (audio_output_t *aout)
> >  static int Start (audio_output_t *aout, audio_sample_format_t *fmt)
> >  {
> >      aout_sys_t *sys = aout->sys;
> > -    char format[5] = "S16N";
> >      unsigned channels;
> > -
> > +    char * format;
> > +    format = var_InheritString(aout, "amem-format");
> > +    int i_format = 0;
> > +    if(!strcmp(format, "S16N"))
> > +    {
> > +        i_format = VLC_CODEC_S16N;
> > +    }
> > +    else if (!strcmp(format, "FL32"))
> > +    {
> > +        i_format = VLC_CODEC_FL32;
> > +    } else
> > +    {
> > +        msg_Warn(aout, "Audio format invalid: %s, amem only supports
> FL32 and S16N, using default: S16N", format);
> > +        i_format = VLC_CODEC_S16N;
> > +        strcpy(format, "S16N");
>
> Do you know for sure that the space allocated by var_InheritString()
> is big enough to hold this string?
>
> > +    }
> >      if (sys->setup != NULL)
> >      {
> >          channels = aout_FormatNbChannels(fmt);
> > -
> >          sys->opaque = sys->setup_opaque;
> > -        if (sys->setup (&sys->opaque, format, &fmt->i_rate, &channels))
> > +        if (sys->setup (&sys->opaque, format, &fmt->i_rate, &channels)
> != 0)
> > +        {
> > +            msg_Err(aout, "Audio format setup failure (audio playback
> skipped)");
>
> You're leaking format here.
>
> >              return VLC_EGENERIC;
> > +        }
> >      }
> >      else
> >      {
> >          fmt->i_rate = sys->rate;
> >          channels = sys->channels;
> >      }
> > +    fmt->i_format = i_format;
> >
> >      /* Initialize volume (in case the UI changed volume before setup) */
> >      sys->ready = true;
> >      if (sys->set_volume != NULL)
> >          sys->set_volume(sys->opaque, sys->volume, sys->mute);
> >
> > -    /* Ensure that format is supported */
> > -    if (fmt->i_rate == 0 || fmt->i_rate > 192000
> > -     || channels == 0 || channels > AOUT_CHAN_MAX
> > -     || strcmp(format, "S16N") /* TODO: amem-format */)
> > +    /* Ensure that format is supported, regarding rate and channels */
> > +    if (fmt->i_rate == 0 || fmt->i_rate > 192000
> > +     || channels == 0 || channels > AOUT_CHAN_MAX)
> >      {
> >          msg_Err (aout, "format not supported: %s, %u channel(s), %u Hz",
> >                   format, channels, fmt->i_rate);
> >          Stop (aout);
> > +        free(format);
> >          return VLC_EGENERIC;
> >      }
> > -
> > +    free(format);
> >      /* channel mapping */
> >      switch (channels)
> >      {
> > @@ -227,7 +244,6 @@ static int Start (audio_output_t *aout,
> audio_sample_format_t *fmt)
> >              vlc_assert_unreachable();
> >      }
> >
> > -    fmt->i_format = VLC_CODEC_S16N;
> >      fmt->i_original_channels = fmt->i_physical_channels;
> >      return VLC_SUCCESS;
> >  }
> > --
> > 1.9.1
> >
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
>
> Best,
> Tristan
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20160613/20b90cec/attachment.html>