[FFmpeg-devel] [PATCH] lavc/videotoolboxenc: Workaround encoder error

[wm4]

On Mon, 21 Mar 2016 19:55:24 +0800
Rick Kern <kernrj at gmail.com> wrote:

> Fixes bug #5352: Error when fetching parameter sets.
> 
> Signed-off-by: Rick Kern <kernrj at gmail.com>

Could use some more explanations. A referenced issue is IMHO not
enough, and the issue tracker might disappear/be reset/be replaced in
the future too.

> ---
>  libavcodec/videotoolboxenc.c | 38 ++++++++++++++++++++++++--------------
>  1 file changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
> index 0791146..a9fa96b 100644
> --- a/libavcodec/videotoolboxenc.c
> +++ b/libavcodec/videotoolboxenc.c
> @@ -194,6 +194,7 @@ static int get_params_size(
>  {
>      size_t total_size = 0;
>      size_t ps_count;
> +    int is_count_bad = 0;
>      size_t i;
>      int status;
>      status = CMVideoFormatDescriptionGetH264ParameterSetAtIndex(vid_fmt,
> @@ -203,27 +204,31 @@ static int get_params_size(
>                                                                  &ps_count,
>                                                                  NULL);
>      if (status) {
> -        av_log(avctx, AV_LOG_ERROR, "Error getting parameter set count: %d\n", status);
> -        return AVERROR_EXTERNAL;
> +        is_count_bad = 1;
> +        ps_count     = 0;
> +        status       = 0;
>      }
>  
> -    for(i = 0; i < ps_count; i++){
> +    for (i = 0; (is_count_bad && !status) || i < ps_count; i++) {
>          const uint8_t *ps;
>          size_t ps_size;
> +
>          status = CMVideoFormatDescriptionGetH264ParameterSetAtIndex(vid_fmt,
>                                                                      i,
>                                                                      &ps,
>                                                                      &ps_size,
>                                                                      NULL,
>                                                                      NULL);
> -        if(status){
> -            av_log(avctx, AV_LOG_ERROR, "Error getting parameter set size for index %zd: %d\n", i, status);
> -            return AVERROR_EXTERNAL;
> -        }
> +        if (status) break;
>  
>          total_size += ps_size + sizeof(start_code);
>      }
>  
> +    if (status && (!i || !is_count_bad)) {

Using the loop variable here seems unnecessarily subtle. (Same in
copy_params_set below.) Why does it even check for i==0, isn't that a
valid index just like i!=0?

> +        av_log(avctx, AV_LOG_ERROR, "Error getting parameter set sizes: %d\n", status);
> +        return AVERROR_EXTERNAL;
> +    }
> +
>      *size = total_size;
>      return 0;
>  }
> @@ -235,6 +240,7 @@ static int copy_param_sets(
>      size_t                      dst_size)
>  {
>      size_t ps_count;
> +    int is_count_bad = 0;
>      int status;
>      size_t offset = 0;
>      size_t i;
> @@ -246,11 +252,13 @@ static int copy_param_sets(
>                                                                  &ps_count,
>                                                                  NULL);
>      if (status) {
> -        av_log(avctx, AV_LOG_ERROR, "Error getting parameter set count for copying: %d\n", status);
> -        return AVERROR_EXTERNAL;
> +        is_count_bad = 1;
> +        ps_count     = 0;
> +        status       = 0;
>      }
>  
> -    for (i = 0; i < ps_count; i++) {
> +
> +    for (i = 0; (is_count_bad && !status) || i < ps_count; i++) {
>          const uint8_t *ps;
>          size_t ps_size;
>          size_t next_offset;
> @@ -261,10 +269,7 @@ static int copy_param_sets(
>                                                                      &ps_size,
>                                                                      NULL,
>                                                                      NULL);
> -        if (status) {
> -            av_log(avctx, AV_LOG_ERROR, "Error getting parameter set data for index %zd: %d\n", i, status);
> -            return AVERROR_EXTERNAL;
> -        }
> +        if (status) break;
>  
>          next_offset = offset + sizeof(start_code) + ps_size;
>          if (dst_size < next_offset) {
> @@ -279,6 +284,11 @@ static int copy_param_sets(
>          offset = next_offset;
>      }
>  
> +    if (status && (!i || !is_count_bad)) {
> +        av_log(avctx, AV_LOG_ERROR, "Error getting parameter set data: %d\n", status);
> +        return AVERROR_EXTERNAL;
> +    }
> +
>      return 0;
>  }
>