| Message ID | 20220525171516.64893-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avutil/avframe: fix channel layout checks in av_frame_copy() | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
ons 2022-05-25 klockan 14:15 -0300 skrev James Almer: > Normally, both the source and dest frame would have only the old API > fields > set, only the new API fields set, or both set. But in some cases, > like when > calling av_frame_ref() using a non reference counted source frame > where only > the old channel layout API fields were populated, the result would be > the dst > frame having both the new and old fields populated. > > This commit takes this into account and fixes the checks by calling > av_channel_layout_compare() only if the source frame has the new API > fields > set, and doing sanity checks for the source frame old API fields if > the new > ones are not set. A bit of context for those wondering: this is part of ensuring melt works both with FFmpeg 5.0 and the new channel layout API. They're still using deprecated things, but one step at a time. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavutil/frame.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) Works /Tomas
On 5/27/2022 5:12 AM, Tomas Härdin wrote: > ons 2022-05-25 klockan 14:15 -0300 skrev James Almer: >> Normally, both the source and dest frame would have only the old API >> fields >> set, only the new API fields set, or both set. But in some cases, >> like when >> calling av_frame_ref() using a non reference counted source frame >> where only >> the old channel layout API fields were populated, the result would be >> the dst >> frame having both the new and old fields populated. >> >> This commit takes this into account and fixes the checks by calling >> av_channel_layout_compare() only if the source frame has the new API >> fields >> set, and doing sanity checks for the source frame old API fields if >> the new >> ones are not set. > > A bit of context for those wondering: this is part of ensuring melt > works both with FFmpeg 5.0 and the new channel layout API. They're > still using deprecated things, but one step at a time. I assume it may be affecting other library users, since it's a matter of passing an AVFrame with no reference counted buffers (nothing in buf[], only data[] pointers filled) and only the old channel layout API fields set to av_frame_ref() or anything that calls it, like avcodec_send_frame(). The amount of users of the new channel layout at this point is probably very small, since it's not present in any release. I can't say how many users passing AVFrames with non reference counted buffers are out there, but it's a supported scenario, so melt may not be the only case. > >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavutil/frame.c | 19 +++++++++++++++---- >> 1 file changed, 15 insertions(+), 4 deletions(-) > > Works > > /Tomas > > _______________________________________________ > 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".
On 5/25/2022 2:15 PM, James Almer wrote: > Normally, both the source and dest frame would have only the old API fields > set, only the new API fields set, or both set. But in some cases, like when > calling av_frame_ref() using a non reference counted source frame where only > the old channel layout API fields were populated, the result would be the dst > frame having both the new and old fields populated. > > This commit takes this into account and fixes the checks by calling > av_channel_layout_compare() only if the source frame has the new API fields > set, and doing sanity checks for the source frame old API fields if the new > ones are not set. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavutil/frame.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) Will apply.
diff --git a/libavutil/frame.c b/libavutil/frame.c index fbb869fffa..0da4bb4768 100644 --- a/libavutil/frame.c +++ b/libavutil/frame.c @@ -727,19 +727,30 @@ static int frame_copy_audio(AVFrame *dst, const AVFrame *src) #if FF_API_OLD_CHANNEL_LAYOUT FF_DISABLE_DEPRECATION_WARNINGS + int channel_layout = dst->ch_layout.order == AV_CHANNEL_ORDER_NATIVE ? + dst->ch_layout.u.mask : 0; if (!channels) { - if (dst->channels != src->channels || - dst->channel_layout != src->channel_layout) - return AVERROR(EINVAL); channels = dst->channels; + channel_layout = dst->channel_layout; planes = planar ? channels : 1; + } + if (!src->ch_layout.nb_channels) { + if (channels != src->channels || + channel_layout != src->channel_layout) + return AVERROR(EINVAL); CHECK_CHANNELS_CONSISTENCY(src); } FF_ENABLE_DEPRECATION_WARNINGS #endif if (dst->nb_samples != src->nb_samples || +#if FF_API_OLD_CHANNEL_LAYOUT + (av_channel_layout_check(&src->ch_layout) && +#endif av_channel_layout_compare(&dst->ch_layout, &src->ch_layout)) +#if FF_API_OLD_CHANNEL_LAYOUT + ) +#endif return AVERROR(EINVAL); for (i = 0; i < planes; i++) @@ -763,7 +774,7 @@ FF_DISABLE_DEPRECATION_WARNINGS else if (dst->nb_samples > 0 && (av_channel_layout_check(&dst->ch_layout) #if FF_API_OLD_CHANNEL_LAYOUT - || dst->channel_layout || dst->channels + || dst->channels > 0 #endif )) return frame_copy_audio(dst, src);
Normally, both the source and dest frame would have only the old API fields set, only the new API fields set, or both set. But in some cases, like when calling av_frame_ref() using a non reference counted source frame where only the old channel layout API fields were populated, the result would be the dst frame having both the new and old fields populated. This commit takes this into account and fixes the checks by calling av_channel_layout_compare() only if the source frame has the new API fields set, and doing sanity checks for the source frame old API fields if the new ones are not set. Signed-off-by: James Almer <jamrial@gmail.com> --- libavutil/frame.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)