Opened 9 days ago

Last modified 9 days ago

#11069 new defect

Setting frame->channel_layout to non zero in av_buffersrc_add_frame_flags(...) without updating frame->channels as well breaks migration to the new channel layout api from application code.

Reported by: Ondrej Popp Owned by:
Priority: normal Component: avfilter
Version: 5.1.4 Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description (last modified by Ondrej Popp)

Hello @jamrial and ffmpeg.

I tried to comment directly on the code on github but that seems to be disabled. So then let's try over here.

Refering to the source code over here,

https://github.com/FFmpeg/FFmpeg/blob/release/5.1/libavfilter/buffersrc.c

in the function av_buffersrc_add_frame_flags(AVFilterContext *ctx, AVFrame *frame, int flags)

line 212,
#if FF_API_OLD_CHANNEL_LAYOUT
FF_DISABLE_DEPRECATION_WARNINGS

if (!frame->channel_layout)

frame->channel_layout = s->ch_layout.order == AV_CHANNEL_ORDER_NATIVE ?

s->ch_layout.u.mask : 0;

FF_ENABLE_DEPRECATION_WARNINGS
#endif

there is a problem that if you set frame->channel_layout you also need to set frame->channels. I encountered this in the context of migrating to the new channel layout api in the source code of the olive video editor. So then you are not supposed to use the old layout api anymore. So frame->channel_layout will then be zero from av_frame_alloc(). And frame->channels as well.

Then the following condition on line 185 is supressed,

if (frame && frame->channel_layout &&

av_get_channel_layout_nb_channels(frame->channel_layout) != frame->channels) {
av_log(ctx, AV_LOG_ERROR, "Layout indicates a different number of channels than actually present\n");
return AVERROR(EINVAL);

}

because frame->channel_layout is zero.

But when it gets to line 212, frame->channel_layout will be set to a non zero value, when s->ch_layout.order == AV_CHANNEL_ORDER_NATIVE, and then the second time it gets to the condition in line 185 it will detect the inconsistency between frame->channel_layout and frame->channels and it will then break off with the corresponding error warning, and thus break the application code.

So this can be fixed in the application code by explicitly setting both the frame->channel_layout and frame->channels to the values of the new layout api, so ch_layout.u.mask and ch_layout.nb_channels but then you can not migrate completely cleanly to the new layout api, because you still have to use the old api. Or it needs to be fixed over there...

Ok that's it. Have a nice day!

Change History (1)

comment:1 by Ondrej Popp, 9 days ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.