[vlc-devel] [PATCH 5/8] opus: factorize error cases in OpenEncoder

[Tristan Matthews]

On Sat, Sep 21, 2013 at 10:54 AM, Rafaël Carré <funman at videolan.org> wrote:

> ---
>  modules/codec/opus.c | 35 ++++++++++++++++-------------------
>  1 file changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/modules/codec/opus.c b/modules/codec/opus.c
> index fa96d2c..2f50b77 100644
> --- a/modules/codec/opus.c
> +++ b/modules/codec/opus.c
> @@ -571,6 +571,9 @@ static int OpenEncoder(vlc_object_t *p_this)
>      if (!sys)
>          return VLC_ENOMEM;
>
> +    sys->buffer = NULL;
> +    sys->enc = NULL;
> +
>      enc->pf_encode_audio = Encode;
>      enc->fmt_in.i_codec = VLC_CODEC_FL32;
>      enc->fmt_in.audio.i_rate = /* Only 48kHz */
> @@ -584,8 +587,7 @@ static int OpenEncoder(vlc_object_t *p_this)
>              &header))
>      {
>          msg_Err(enc, "Failed to prepare header.");
> -        free(sys);
> -        return VLC_ENOMEM;
> +        goto error;
>      }
>
>      /* needed for max encoded size calculation */
> @@ -601,8 +603,8 @@ static int OpenEncoder(vlc_object_t *p_this)
>      if (err != OPUS_OK)
>      {
>          msg_Err(enc, "Could not create encoder: error %d", err);
> -        free(sys);
> -        return VLC_EGENERIC;
> +        sys->enc = NULL;
> +        goto error;
>      }
>
>      /* TODO: vbr, bitrate, fec */
> @@ -612,11 +614,7 @@ static int OpenEncoder(vlc_object_t *p_this)
>      enc->p_sys = sys;
>      sys->buffer = malloc(OPUS_FRAME_SIZE * header.channels *
> sizeof(float));
>      if (!sys->buffer)
> -    {
> -        opus_multistream_encoder_destroy(sys->enc);
> -        free(sys);
> -        return VLC_ENOMEM;
> -    }
> +        goto error;
>
>      sys->i_nb_samples = 0;
>
> @@ -634,10 +632,7 @@ static int OpenEncoder(vlc_object_t *p_this)
>                            &enc->fmt_out.i_extra, &header))
>      {
>          msg_Err(enc, "Failed to write header.");
> -        free(sys->buffer);
> -        opus_multistream_encoder_destroy(sys->enc);
> -        free(sys);
> -        return VLC_ENOMEM;
> +        goto error;
>      }
>
>      if (sys->i_samples_delay > 0)
> @@ -646,12 +641,7 @@ static int OpenEncoder(vlc_object_t *p_this)
>              enc->fmt_out.audio.i_channels;
>          sys->padding = block_Alloc(padding_samples * sizeof(float));
>          if (!sys->padding)
> -        {
> -            free(sys->buffer);
> -            opus_multistream_encoder_destroy(sys->enc);
> -            free(sys);
> -            return VLC_ENOMEM;
> -        }
> +            goto error;
>          sys->padding->i_nb_samples = sys->i_samples_delay;
>          float *pad_ptr = (float *) sys->padding->p_buffer;
>          memset(pad_ptr, 0, padding_samples * sizeof(float));
> @@ -662,6 +652,13 @@ static int OpenEncoder(vlc_object_t *p_this)
>      }
>
>      return VLC_SUCCESS;
> +
> +error:
> +    if (sys->enc)
> +        opus_multistream_encoder_destroy(sys->enc);
> +    free(sys->buffer);
> +    free(sys);
> +    return VLC_EGENERIC;
>  }
>

The only issue I see here is that we're no longer distinguishing between
out of memory errors and generic error cases. Maybe that distinction isn't
so important in this situation.

In any case, the patchset looks good, should i resend with your changes
merged?

Best,
Tristan

-- 
Tristan Matthews
web: http://tristanswork.blogspot.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20130922/48294f79/attachment.html>