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

[Richard Kern]

> On Mar 25, 2016, at 12:45 AM, wm4 <nfxjfg at googlemail.com> wrote:
> 
> On Mon, 21 Mar 2016 19:55:24 +0800
> Rick Kern <kernrj at gmail.com <mailto:kernrj at gmail.com>> wrote:
> 
>> Fixes bug #5352: Error when fetching parameter sets.
>> 
>> Signed-off-by: Rick Kern <kernrj at gmail.com <mailto: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?
When getting ps_count fails, it depends on getting a failure from CMVideoFormatDescriptionGetH264ParameterSetAtIndex() to stop looping. It fails on i == 0 because it didn’t get anything. I’ll make that clearer.

> 
>> +        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;
>> }
>> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org <mailto:ffmpeg-devel at ffmpeg.org>
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>