[vlc-devel] [PATCH] transcode: video: refactor error handling

[Alexandre Janniaux]

Hi,

thank you for review,

On Fri, Jun 26, 2020 at 02:04:03PM +0200, Thomas Guillem wrote:
>
>
> On Thu, Jun 25, 2020, at 14:19, Alexandre Janniaux wrote:
> > The same error handling path is duplicated at multiple location. Move it
> > after function success and jump to it when necessary instead.
> > ---
> >  modules/stream_out/transcode/video.c | 41 +++++++++-------------------
> >  1 file changed, 13 insertions(+), 28 deletions(-)
> >
> > diff --git a/modules/stream_out/transcode/video.c
> > b/modules/stream_out/transcode/video.c
> > index 1d5db5b84d..094c1a357f 100644
> > --- a/modules/stream_out/transcode/video.c
> > +++ b/modules/stream_out/transcode/video.c
> > @@ -226,13 +226,8 @@ int transcode_video_init( sout_stream_t *p_stream,
> > const es_format_t *p_fmt,
> >
> >      struct encoder_owner *p_enc_owner = (struct
> > encoder_owner*)sout_EncoderCreate(p_stream, sizeof(struct
> > encoder_owner));
> >      if ( unlikely(p_enc_owner == NULL))
> > -    {
> > -        module_unneed( id->p_decoder, id->p_decoder->p_module );
> > -        id->p_decoder->p_module = NULL;
> > -        es_format_Clean( &id->decoder_out );
> > -        es_format_Clean( &encoder_tested_fmt_in );
> > -        return VLC_EGENERIC;
> > -    }
> > +       goto error;
> > +
> >      p_enc_owner->id = id;
> >      p_enc_owner->enc.cbs = &encoder_video_transcode_cbs;
> >
> > @@ -241,33 +236,16 @@ int transcode_video_init( sout_stream_t
> > *p_stream, const es_format_t *p_fmt,
> >                                  &id->p_decoder->fmt_in,
> >                                  id->p_decoder->fmt_out.i_codec,
> >                                  &encoder_tested_fmt_in ) )
> > -    {
> > -        module_unneed( id->p_decoder, id->p_decoder->p_module );
> > -        id->p_decoder->p_module = NULL;
> > -        es_format_Clean( &id->decoder_out );
> > -        es_format_Clean( &encoder_tested_fmt_in );
> > -        return VLC_EGENERIC;
> > -    }
> > +       goto error;
> >
> >      p_enc_owner = (struct encoder_owner *)sout_EncoderCreate(p_stream,
> > sizeof(struct encoder_owner));
> >      if ( unlikely(p_enc_owner == NULL))
> > -    {
> > -        module_unneed( id->p_decoder, id->p_decoder->p_module );
> > -        id->p_decoder->p_module = NULL;
> > -        es_format_Clean( &encoder_tested_fmt_in );
> > -        es_format_Clean( &id->decoder_out );
> > -        return VLC_EGENERIC;
> > -    }
> > +       goto error;
> >
> >      id->encoder = transcode_encoder_new( &p_enc_owner->enc,
> > &encoder_tested_fmt_in );
> >      if( !id->encoder )
>
> No need to clean p_enc_owner here?

actually no, this part would benefit from comments but
transcode_encoder_new and transcode_encoder_test are consuming
the encoder_owner instance in order to produce an object of
type transcode_encoder_t, so error handling is already shipped
within the function calls.

I can add documentation in some other patchset if you want.

Regards,
--
Alexandre Janniaux
Videolabs