> On Mar 25, 2016, at 12:45 AM, wm4 <nfx...@googlemail.com> wrote:
>
> On Mon, 21 Mar 2016 19:55:24 +0800
> Rick Kern <ker...@gmail.com <mailto:ker...@gmail.com>> wrote:
>
>> Fixes bug #5352: Error when fetching parameter sets.
>>
>> Signed-off-by: Rick Kern <ker...@gmail.com <mailto:ker...@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@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
