Re: [FFmpeg-devel] [PATCH 001/289 v4] Add a new channel layout API

[Marton Balint <cus@passwd.hu>]

On Mon, 17 Jan 2022, James Almer wrote:

[...]

> -static const char *get_channel_name(int channel_id)
> +static const char *get_channel_name(enum AVChannel channel_id)
> {
> -    if (channel_id < 0 || channel_id >= FF_ARRAY_ELEMS(channel_names))
> +    if ((unsigned) channel_id >= FF_ARRAY_ELEMS(channel_names) ||
> +        !channel_names[channel_id].name)
>         return NULL;
>     return channel_names[channel_id].name;
> }
>
> -static const struct {
> +void av_channel_name_bprint(AVBPrint *bp, enum AVChannel channel_id)
> +{
> +    av_bprint_clear(bp);

Clearing should not be done here. Maybe the user wants to construct a
string, and the channel name is only a part of it. If not, the user can
clear the bprint buffer himself before calling this.

> +
> +    if ((unsigned)channel_id < FF_ARRAY_ELEMS(channel_names))
> +        av_bprintf(bp, "%s", channel_names[channel_id].name);
> +    else
> +        av_bprintf(bp, "USR%d", channel_id);
> +}
> +
> +int av_channel_name(char *buf, size_t buf_size, enum AVChannel channel_id)
> +{
> +    AVBPrint bp;
> +
> +    if (!buf && buf_size)
> +        return AVERROR(EINVAL);
> +
> +    av_bprint_init_for_buffer(&bp, buf, buf_size);
> +    av_channel_name_bprint(&bp, channel_id);
> +
> +    return bp.len;
> +}
> +
> +void av_channel_description_bprint(AVBPrint *bp, enum AVChannel channel_id)
> +{
> +    av_bprint_clear(bp);

Same here.

> +
> +    if ((unsigned)channel_id < FF_ARRAY_ELEMS(channel_names))
> +        av_bprintf(bp, "%s", channel_names[channel_id].description);
> +    else
> +        av_bprintf(bp, "user %d", channel_id);
> +}
> +
> +int av_channel_description(char *buf, size_t buf_size, enum AVChannel channel_id)
> +{
> +    AVBPrint bp;
> +
> +    if (!buf && buf_size)
> +        return AVERROR(EINVAL);
> +
> +    av_bprint_init_for_buffer(&bp, buf, buf_size);
> +    av_channel_description_bprint(&bp, channel_id);
> +
> +    return bp.len;
> +}

[...]

> +int av_channel_layout_from_string(AVChannelLayout *channel_layout,
> +                                  const char *str)
> +{
> +    int i;
> +    int channels = 0, native = 1;
> +    enum AVChannel highest_channel = AV_CHAN_NONE;
> +    const char *dup = str;
> +    char *end;
> +    uint64_t mask = 0;
> +
> +    /* channel layout names */
> +    for (i = 0; i < FF_ARRAY_ELEMS(channel_layout_map); i++) {
> +        if (channel_layout_map[i].name && !strcmp(str, channel_layout_map[i].name)) {
> +            *channel_layout = channel_layout_map[i].layout;
> +            return 0;
> +        }
> +    }
> +
> +    /* channel names */
> +    while (*dup) {
> +        char *chname = av_get_token(&dup, "+");
> +        if (!chname)
> +            return AVERROR(ENOMEM);
> +        if (*dup)
> +            dup++; // skip separator
> +        for (i = 0; i < FF_ARRAY_ELEMS(channel_names); i++) {
> +            if (channel_names[i].name && !strcmp(chname, channel_names[i].name)) {
> +                if (i < highest_channel || mask & (1ULL << i))
> +                    native = 0; // Not a native layout, use a custom one
> +                highest_channel = i;
> +                mask |= 1ULL << i;
> +                break;
> +            }
> +        }
> +
> +        if (i >= FF_ARRAY_ELEMS(channel_names)) {
> +            char *endptr = chname;
> +            enum AVChannel id = AV_CHAN_NONE;
> +
> +            if (!strncmp(chname, "USR", 3)) {
> +                const char *p = chname + 3;
> +                id = strtol(p, &endptr, 0);
> +            }
> +            if (id < 0 || *endptr) {
> +                native = 0; // Unknown channel name
> +                channels = 0;
> +                mask = 0;
> +                av_free(chname);
> +                break;
> +            }
> +            if (id > 63)
> +                native = 0; // Not a native layout, use a custom one
> +            else {
> +                if (id < highest_channel || mask & (1ULL << id))
> +                    native = 0; // Not a native layout, use a custom one
> +                highest_channel = id;
> +                mask |= 1ULL << id;
> +            }
> +        }
> +        channels++;
> +        av_free(chname);
> +    }
> +    if (mask && native) {
> +        av_channel_layout_from_mask(channel_layout, mask);
> +        return 0;
> +    }
> +
> +    /* custom layout of channel names */
> +    if (channels && !native) {
> +        int idx = 0;
> +
> +        channel_layout->u.map = av_calloc(channels, sizeof(*channel_layout->u.map));
> +        if (!channel_layout->u.map)
> +            return AVERROR(ENOMEM);
> +
> +        channel_layout->order = AV_CHANNEL_ORDER_CUSTOM;
> +        channel_layout->nb_channels = channels;
> +
> +        dup = str;
> +        while (*dup) {
> +            char *chname = av_get_token(&dup, "+");
> +            if (!chname) {
> +                av_freep(&channel_layout->u.map);
> +                return AVERROR(ENOMEM);
> +            }
> +            if (*dup)
> +                dup++; // skip separator
> +            for (i = 0; i < FF_ARRAY_ELEMS(channel_names); i++) {
> +                if (channel_names[i].name && !strcmp(chname, channel_names[i].name)) {
> +                    channel_layout->u.map[idx++].id = i;
> +                    break;
> +                }
> +            }
> +            if (i >= FF_ARRAY_ELEMS(channel_names)) {
> +                const char *p = chname + 3;
> +                channel_layout->u.map[idx++].id = strtol(p, NULL, 0);
> +            }
> +            av_free(chname);
> +        }
> +
> +        return 0;
> +    }
> +
> +    /* channel layout mask */
> +    if (!strncmp(str, "0x", 2) && sscanf(str + 2, "%"SCNx64, &mask) == 1) {
> +        av_channel_layout_from_mask(channel_layout, mask);
> +        return 0;
> +    }
> +
> +    errno = 0;
> +    channels = strtol(str, &end, 10);
> +
> +    /* number of channels */
> +    if (!errno && *end == 'c' && !*(end + 1) && channels >= 0) {
> +        av_channel_layout_default(channel_layout, channels);
> +        return 0;
> +    }
> +
> +    /* number of unordered channels */
> +    if (!errno && (!*end || (*end == 'C' && !*(end + 1)) || av_strnstr(str, "channels", strlen(str)))
> +        && channels >= 0) {
> +        channel_layout->order = AV_CHANNEL_ORDER_UNSPEC;
> +        channel_layout->nb_channels = channels;
> +        return 0;
> +    }
> +
> +    return AVERROR_INVALIDDATA;

rather AVERROR(EINVAL), no?

> +}
> +
> +void av_channel_layout_uninit(AVChannelLayout *channel_layout)
> +{
> +    if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM)
> +        av_freep(&channel_layout->u.map);
> +    memset(channel_layout, 0, sizeof(*channel_layout));
> +}
> +
> +int av_channel_layout_copy(AVChannelLayout *dst, const AVChannelLayout *src)
> +{
> +    av_channel_layout_uninit(dst);
> +    *dst = *src;
> +    if (src->order == AV_CHANNEL_ORDER_CUSTOM) {
> +        dst->u.map = av_malloc_array(src->nb_channels, sizeof(*dst->u.map));
> +        if (!dst->u.map)
> +            return AVERROR(ENOMEM);
> +        memcpy(dst->u.map, src->u.map, src->nb_channels * sizeof(*src->u.map));
> +    }
> +    return 0;
> +}
> +
> +int av_channel_layout_describe_bprint(const AVChannelLayout *channel_layout,
> +                                      AVBPrint *bp)
> +{
> +    int i;
> +
> +    av_bprint_clear(bp);

Same as above, clearing should not be done.

Thanks,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".