| Message ID | 20220824084318.333-16-anton@khirnov.net |
|---|---|
| State | Accepted |
| Commit | 6def44128afedcfbcad98ca00e209345875fac45 |
| Headers | show |
| Series | [FFmpeg-devel,01/18] tests/fate/mov: add a test for dv audio demuxed through dv demuxer | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | fail | Make fate failed |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | fail | Make fate failed |
Anton Khirnov: > Demuxers are not supposed to update AVCodecParameters after the stream > was seen by the caller. This value is not important enough to support > dynamic updates for. > --- > libavformat/dv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/dv.c b/libavformat/dv.c > index f65c2d596f..9c8b0a262c 100644 > --- a/libavformat/dv.c > +++ b/libavformat/dv.c > @@ -280,6 +280,7 @@ static int dv_extract_audio_info(DVDemuxContext *c, const uint8_t *frame) > c->ast[i]->codecpar->codec_id = AV_CODEC_ID_PCM_S16LE; > c->ast[i]->codecpar->ch_layout = (AVChannelLayout)AV_CHANNEL_LAYOUT_STEREO; > c->ast[i]->start_time = 0; > + c->ast[i]->codecpar->bit_rate = 2 * dv_audio_frequency[freq] * 16; > > c->audio_pkt[i].size = 0; > c->audio_pkt[i].data = c->audio_buf[i]; > @@ -290,7 +291,6 @@ static int dv_extract_audio_info(DVDemuxContext *c, const uint8_t *frame) > c->audio_pkt[i].pos = -1; > } > c->ast[i]->codecpar->sample_rate = dv_audio_frequency[freq]; > - c->ast[i]->codecpar->bit_rate = 2 * dv_audio_frequency[freq] * 16; > } > c->ach = ach; > LGTM. - Andreas
Anton Khirnov: > Demuxers are not supposed to update AVCodecParameters after the stream > was seen by the caller. This value is not important enough to support > dynamic updates for. > --- > libavformat/dv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/dv.c b/libavformat/dv.c > index f65c2d596f..9c8b0a262c 100644 > --- a/libavformat/dv.c > +++ b/libavformat/dv.c > @@ -280,6 +280,7 @@ static int dv_extract_audio_info(DVDemuxContext *c, const uint8_t *frame) > c->ast[i]->codecpar->codec_id = AV_CODEC_ID_PCM_S16LE; > c->ast[i]->codecpar->ch_layout = (AVChannelLayout)AV_CHANNEL_LAYOUT_STEREO; > c->ast[i]->start_time = 0; > + c->ast[i]->codecpar->bit_rate = 2 * dv_audio_frequency[freq] * 16; > > c->audio_pkt[i].size = 0; > c->audio_pkt[i].data = c->audio_buf[i]; > @@ -290,7 +291,6 @@ static int dv_extract_audio_info(DVDemuxContext *c, const uint8_t *frame) > c->audio_pkt[i].pos = -1; > } > c->ast[i]->codecpar->sample_rate = dv_audio_frequency[freq]; > - c->ast[i]->codecpar->bit_rate = 2 * dv_audio_frequency[freq] * 16; > } > c->ach = ach; > FYI: dv_extract_video_info() also sets bit_rate (and avg_frame_rate and sample_aspect_ratio) on every packet. - Andreas
Quoting Andreas Rheinhardt (2022-08-24 16:33:22) > Anton Khirnov: > > Demuxers are not supposed to update AVCodecParameters after the stream > > was seen by the caller. This value is not important enough to support > > dynamic updates for. > > --- > > libavformat/dv.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavformat/dv.c b/libavformat/dv.c > > index f65c2d596f..9c8b0a262c 100644 > > --- a/libavformat/dv.c > > +++ b/libavformat/dv.c > > @@ -280,6 +280,7 @@ static int dv_extract_audio_info(DVDemuxContext *c, const uint8_t *frame) > > c->ast[i]->codecpar->codec_id = AV_CODEC_ID_PCM_S16LE; > > c->ast[i]->codecpar->ch_layout = (AVChannelLayout)AV_CHANNEL_LAYOUT_STEREO; > > c->ast[i]->start_time = 0; > > + c->ast[i]->codecpar->bit_rate = 2 * dv_audio_frequency[freq] * 16; > > > > c->audio_pkt[i].size = 0; > > c->audio_pkt[i].data = c->audio_buf[i]; > > @@ -290,7 +291,6 @@ static int dv_extract_audio_info(DVDemuxContext *c, const uint8_t *frame) > > c->audio_pkt[i].pos = -1; > > } > > c->ast[i]->codecpar->sample_rate = dv_audio_frequency[freq]; > > - c->ast[i]->codecpar->bit_rate = 2 * dv_audio_frequency[freq] * 16; > > } > > c->ach = ach; > > > > FYI: dv_extract_video_info() also sets bit_rate (and avg_frame_rate and > sample_aspect_ratio) on every packet. I know, it's just that I already spent way more time on this than I intended, given how nobody really cares about DV (so much of the code is blamed directly to original commits from 2003). And while dropping bitrate is rather uncontroversial, since it's informational only, the framerate and SAR might actually be used for presentation, so handling them requires more care. I've considered using AVStream.event_flags for signalling updated values to the user, but then it might become tricky to synchronize the values seen by the user with the packets returned from the demuxer (given the presence of parsers).
diff --git a/libavformat/dv.c b/libavformat/dv.c index f65c2d596f..9c8b0a262c 100644 --- a/libavformat/dv.c +++ b/libavformat/dv.c @@ -280,6 +280,7 @@ static int dv_extract_audio_info(DVDemuxContext *c, const uint8_t *frame) c->ast[i]->codecpar->codec_id = AV_CODEC_ID_PCM_S16LE; c->ast[i]->codecpar->ch_layout = (AVChannelLayout)AV_CHANNEL_LAYOUT_STEREO; c->ast[i]->start_time = 0; + c->ast[i]->codecpar->bit_rate = 2 * dv_audio_frequency[freq] * 16; c->audio_pkt[i].size = 0; c->audio_pkt[i].data = c->audio_buf[i]; @@ -290,7 +291,6 @@ static int dv_extract_audio_info(DVDemuxContext *c, const uint8_t *frame) c->audio_pkt[i].pos = -1; } c->ast[i]->codecpar->sample_rate = dv_audio_frequency[freq]; - c->ast[i]->codecpar->bit_rate = 2 * dv_audio_frequency[freq] * 16; } c->ach = ach;