[FFmpeg-devel] [PATCH 3/8] ffmpeg: add support for muxing AVStreamGroups

[Anton Khirnov]

Quoting James Almer (2023-12-11 13:46:36)
> AVStreamGroup.type is not setteable through AVOptions, but it of course 
> needs to be supported by the CLI. So i catch it and remove it from the 
> dict that will be used for avformat_stream_group_create().
> 
> I can change the comment to "'type' is not a user settable AVOption".

That seems better, thanks.

> > 3) Print the string, not the index.
> > 
> >> +                ret = AVERROR(EINVAL);
> >> +                goto end;
> >> +            }
> >> +
> >> +            ret = av_opt_eval_int(&pclass, opts, e->value, &type);
> >> +            if (ret < 0 || type == AV_STREAM_GROUP_PARAMS_NONE) {
> >> +                av_log(mux, AV_LOG_ERROR, "Invalid group type \"%s\"\n", e->value);
> >> +                goto end;
> >> +            }
> >> +
> >> +            av_dict_copy(&tmp, dict, 0);
> >> +            stg = avformat_stream_group_create(oc, type, &tmp);
> >> +            if (!stg) {
> >> +                ret = AVERROR(ENOMEM);
> >> +                goto end;
> >> +            }
> >> +            av_dict_set(&tmp, "type", NULL, 0);
> >> +
> >> +            e = NULL;
> >> +            while (e = av_dict_get(dict, "st", e, 0)) {
> >> +                unsigned int idx = strtol(e->value, NULL, 0);
> >> +                if (idx >= oc->nb_streams) {
> >> +                    av_log(mux, AV_LOG_ERROR, "Invalid stream index %d\n", idx);
> >> +                    ret = AVERROR(EINVAL);
> >> +                    goto end;
> >> +                }
> > 
> > This block seems confused about signedness of e->value.
> 
> You mean change %d to %u?

I mean strtol will parse the string into a signed number, then you
assign the result into unsigned, and print it as signed. It's probably
more user-friendly to keep parsing it as signed, and add a check for
idx >= 0.

-- 
Anton Khirnov