[FFmpeg-trac] #11069(avfilter:new): 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.

[FFmpeg]

#11069: 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.
-------------------------------------+------------------------------------
             Reporter:  Ondrej Popp  |                    Owner:  (none)
                 Type:  defect       |                   Status:  new
             Priority:  normal       |                Component:  avfilter
              Version:  5.1.4        |               Resolution:
             Keywords:               |               Blocked By:
             Blocking:               |  Reproduced by developer:  0
Analyzed by developer:  0            |
-------------------------------------+------------------------------------
Description changed by Ondrej Popp:

Old description:

> 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, 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!

New description:

 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!

--
-- 
Ticket URL: <https://trac.ffmpeg.org/ticket/11069#comment:1>
FFmpeg <https://ffmpeg.org>
FFmpeg issue tracker