On Thu, May 30, 2024 at 09:33:51PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Thu, May 30, 2024 at 08:07:48PM +0200, Andreas Rheinhardt wrote:
> >> Michael Niedermayer:
> >>> On Thu, May 30, 2024 at 07:53:42PM +0200, Andreas Rheinhardt wrote:
> >>>> Michael Niedermayer:
> >>>>> On Thu, May 30, 2024 at 02:14:20AM +0200, Andreas Rheinhardt wrote:
> >>>>>> Forgotten in 65ddc74988245a01421a63c5cffa4d900c47117c.
> >>>>>>
> >>>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
> >>>>>> ---
> >>>>>> libavformat/nutdec.c | 14 ++++----------
> >>>>>> 1 file changed, 4 insertions(+), 10 deletions(-)
> >>>>>>
> >>>>>> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
> >>>>>> index 0bb7f154db..34b7e3cb9a 100644
> >>>>>> --- a/libavformat/nutdec.c
> >>>>>> +++ b/libavformat/nutdec.c
> >>>>>> @@ -881,8 +881,6 @@ static int read_sm_data(AVFormatContext *s,
> >>>>>> AVIOContext *bc, AVPacket *pkt, int
> >>>>>> int count = ffio_read_varlen(bc);
> >>>>>> int skip_start = 0;
> >>>>>> int skip_end = 0;
> >>>>>> - int channels = 0;
> >>>>>> - int64_t channel_layout = 0;
> >>>>>> int sample_rate = 0;
> >>>>>> int width = 0;
> >>>>>> int height = 0;
> >>>>>> @@ -930,7 +928,7 @@ static int read_sm_data(AVFormatContext *s,
> >>>>>> AVIOContext *bc, AVPacket *pkt, int
> >>>>>> AV_WB64(dst, v64);
> >>>>>> dst += 8;
> >>>>>> } else if (!strcmp(name, "ChannelLayout") && value_len ==
> >>>>>> 8) {
> >>>>>> - channel_layout = avio_rl64(bc);
> >>>>>> + // Ignored
> >>>>>> continue;
> >>>>>> } else {
> >>>>>> av_log(s, AV_LOG_WARNING, "Unknown data %s / %s\n",
> >>>>>> name, type_str);
> >>>>>> @@ -952,7 +950,7 @@ static int read_sm_data(AVFormatContext *s,
> >>>>>> AVIOContext *bc, AVPacket *pkt, int
> >>>>>> } else if (!strcmp(name, "SkipEnd")) {
> >>>>>> skip_end = value;
> >>>>>> } else if (!strcmp(name, "Channels")) {
> >>>>>> - channels = value;
> >>>>>> + // Ignored
> >>>>>> } else if (!strcmp(name, "SampleRate")) {
> >>>>>> sample_rate = value;
> >>>>>> } else if (!strcmp(name, "Width")) {
> >>>>>> @@ -965,18 +963,14 @@ static int read_sm_data(AVFormatContext *s,
> >>>>>> AVIOContext *bc, AVPacket *pkt, int
> >>>>>> }
> >>>>>> }
> >>>>>>
> >>>>>> - if (channels || channel_layout || sample_rate || width || height)
> >>>>>> {
> >>>>>> - uint8_t *dst = av_packet_new_side_data(pkt,
> >>>>>> AV_PKT_DATA_PARAM_CHANGE, 28);
> >>>>>> + if (sample_rate || width || height) {
> >>>>>> + uint8_t *dst = av_packet_new_side_data(pkt,
> >>>>>> AV_PKT_DATA_PARAM_CHANGE, 16);
> >>>>>> if (!dst)
> >>>>>> return AVERROR(ENOMEM);
> >>>>>> bytestream_put_le32(&dst,
> >>>>>>
> >>>>>> AV_SIDE_DATA_PARAM_CHANGE_SAMPLE_RATE*(!!sample_rate) +
> >>>>>>
> >>>>>> AV_SIDE_DATA_PARAM_CHANGE_DIMENSIONS*(!!(width|height))
> >>>>>> );
> >>>>>> - if (channels)
> >>>>>> - bytestream_put_le32(&dst, channels);
> >>>>>> - if (channel_layout)
> >>>>>> - bytestream_put_le64(&dst, channel_layout);
> >>>>>> if (sample_rate)
> >>>>>> bytestream_put_le32(&dst, sample_rate);
> >>>>>> if (width || height){
> >>>>>
> >>>>> This would break mid stream changes to the channel layout & channels
> >>>>> when it
> >>>>> is carried at format level only
> >>>>>
> >>>>> The commit message also does not adequately explain why such mid stream
> >>>>> changes
> >>>>> are ignored
> >>>>>
> >>>>
> >>>> Mid-stream changes like this have been deprecated in
> >>>> 09b5d3fb44ae1036700f80c8c80b15e9074c58c3;
> >>>> 65ddc74988245a01421a63c5cffa4d900c47117c removed it, but only
> >>>> incompletely: The side data flags for channel count and channel layout
> >>>> changes were no longer written (in fact, they were removed from
> >>>> packet.h), yet it still wrote the rest of the side data as if these
> >>>> flags existed and had been written. That is the inconsistency this
> >>>> commit addresses. It does not address whether channel count/layout
> >>>> updates should have been removed, because that has already happened.
> >>>
> >>> i honestly belive that we should support changing channel(layout) for
> >>> cases like PCM in nut
> >>>
> >>
> >> That is orthogonal to this patch (which just wants to not create
> >> inconsistent side data).
> >
> > You can fix the inconsistency in 2 directions
> > 1. remove everyting
> > 2. add the code back that made it inconsistant
> >
> > This line between these 2 points is not orthogonal to what this patch
> > changes
> > It also is not orthoginal to supporting PCM channel changes in NUT
> > nor is the change this patch does from our current state orthogonal
> > to what would be needed to support channel changes
> >
> > IMHO, decide on what the end goal is and work toward it. Not just
> > make something consistent even when its a direction that might be suboptimal
> >
>
> We have a release that is able to create nonsense side data; this needs
> to be fixed and for this to be fixed we need a patch to backport.what do you mean by "nonsense side data" ? I would assume the side data is what was documented previously thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are too smart to engage in politics are punished by being governed by those who are dumber. -- Plato
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
