| Message ID | bb07ce7c-4ebb-32cb-25d8-da21ff9d3f26@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Feb 01, 2018 at 07:52:55PM +0530, Gyan Doshi wrote: > On 1/29/2018 6:53 PM, Gyan Doshi wrote: > > > >On 1/29/2018 3:47 PM, Carl Eugen Hoyos wrote: > > > >>How did you test this patch? > > > >By remuxing with patched ffmpeg and then checking with ffprobe, but not > >ffplay! > ... > >Sorry, I'll rework and submit. > > Revised and tested patch attached. > mpegenc.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > 21908814c802dffad710828d1e5ec27168586636 v2-0001-avformat-mpegenc-accept-PCM_DVD-streams.patch > From 78757309bbfbc9d9e48aabd6babc9529f83d5950 Mon Sep 17 00:00:00 2001 > From: Gyan Doshi <gyandoshi@gmail.com> > Date: Thu, 1 Feb 2018 19:12:03 +0530 > Subject: [PATCH v2] avformat/mpegenc - accept PCM_DVD streams > > PCM_S16BE stream packets in MPEG-PS have a 3-byte header > and recognized as PCM_DVD by the demuxer which prevents > their proper remuxing in MPEG-1/2 PS. its probably a good idea to add a fate test for this too. (could be in a seperate patch) [...]
On Thu, Feb 01, 2018 at 07:52:55PM +0530, Gyan Doshi wrote: > On 1/29/2018 6:53 PM, Gyan Doshi wrote: > > > >On 1/29/2018 3:47 PM, Carl Eugen Hoyos wrote: > > > >>How did you test this patch? > > > >By remuxing with patched ffmpeg and then checking with ffprobe, but not > >ffplay! > ... > >Sorry, I'll rework and submit. > > Revised and tested patch attached. > mpegenc.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > 21908814c802dffad710828d1e5ec27168586636 v2-0001-avformat-mpegenc-accept-PCM_DVD-streams.patch > From 78757309bbfbc9d9e48aabd6babc9529f83d5950 Mon Sep 17 00:00:00 2001 > From: Gyan Doshi <gyandoshi@gmail.com> > Date: Thu, 1 Feb 2018 19:12:03 +0530 > Subject: [PATCH v2] avformat/mpegenc - accept PCM_DVD streams > > PCM_S16BE stream packets in MPEG-PS have a 3-byte header > and recognized as PCM_DVD by the demuxer which prevents > their proper remuxing in MPEG-1/2 PS. > --- > libavformat/mpegenc.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c > index c77c3dfe41..598df662f4 100644 > --- a/libavformat/mpegenc.c > +++ b/libavformat/mpegenc.c > @@ -353,7 +353,8 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx) > if (!s->is_mpeg2 && > (st->codecpar->codec_id == AV_CODEC_ID_AC3 || > st->codecpar->codec_id == AV_CODEC_ID_DTS || > - st->codecpar->codec_id == AV_CODEC_ID_PCM_S16BE)) > + st->codecpar->codec_id == AV_CODEC_ID_PCM_S16BE || > + st->codecpar->codec_id == AV_CODEC_ID_PCM_DVD)) > av_log(ctx, AV_LOG_WARNING, > "%s in MPEG-1 system streams is not widely supported, " > "consider using the vob or the dvd muxer " > @@ -363,7 +364,10 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx) > stream->id = ac3_id++; > } else if (st->codecpar->codec_id == AV_CODEC_ID_DTS) { > stream->id = dts_id++; > - } else if (st->codecpar->codec_id == AV_CODEC_ID_PCM_S16BE) { > + } else if (st->codecpar->codec_id == AV_CODEC_ID_PCM_S16BE || > + st->codecpar->codec_id == AV_CODEC_ID_PCM_DVD) { > + if (st->codecpar->codec_id == AV_CODEC_ID_PCM_DVD) > + av_log(ctx, AV_LOG_WARNING, "Only 16-bit LPCM streams are supported.\n"); > stream->id = lpcm_id++; > for (j = 0; j < 4; j++) { > if (lpcm_freq_tab[j] == st->codecpar->sample_rate) > @@ -1150,8 +1154,17 @@ static int mpeg_mux_write_packet(AVFormatContext *ctx, AVPacket *pkt) > return AVERROR(ENOMEM); > pkt_desc->pts = pts; > pkt_desc->dts = dts; > + > + if (st->codecpar->codec_id == AV_CODEC_ID_PCM_DVD) { > + /* Skip first 3 bytes of packet data, which comprise PCM header > + and will be written fresh by this muxer. */ > + buf += 3; > + size -= 3; Can this be reached with size < 3 ? if so it would probably do something bad > + } > + > pkt_desc->unwritten_size = > pkt_desc->size = size; > + > if (!stream->predecode_packet) > stream->predecode_packet = pkt_desc; > stream->next_packet = &pkt_desc->next; > -- > 2.11.1.windows.1 > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 2/3/2018 3:59 AM, Michael Niedermayer wrote: >> PCM_S16BE stream packets in MPEG-PS have a 3-byte header >> and recognized as PCM_DVD by the demuxer which prevents >> their proper remuxing in MPEG-1/2 PS. > > its probably a good idea to add a fate test for this too. > (could be in a seperate patch) What type of test do you recommend? We'll need to check the decoded packets, so framecrc? Regards, Gyan
On Thu, Feb 08, 2018 at 02:04:57PM +0530, Gyan Doshi wrote: > > > On 2/3/2018 3:59 AM, Michael Niedermayer wrote: > > >>PCM_S16BE stream packets in MPEG-PS have a 3-byte header > >>and recognized as PCM_DVD by the demuxer which prevents > >>their proper remuxing in MPEG-1/2 PS. > > > >its probably a good idea to add a fate test for this too. > >(could be in a seperate patch) > > What type of test do you recommend? > We'll need to check the decoded packets, so framecrc? framecrc as well as the md5 of the muxed file or something like that seems a possibility, yes thx [...]
diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c index c77c3dfe41..598df662f4 100644 --- a/libavformat/mpegenc.c +++ b/libavformat/mpegenc.c @@ -353,7 +353,8 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx) if (!s->is_mpeg2 && (st->codecpar->codec_id == AV_CODEC_ID_AC3 || st->codecpar->codec_id == AV_CODEC_ID_DTS || - st->codecpar->codec_id == AV_CODEC_ID_PCM_S16BE)) + st->codecpar->codec_id == AV_CODEC_ID_PCM_S16BE || + st->codecpar->codec_id == AV_CODEC_ID_PCM_DVD)) av_log(ctx, AV_LOG_WARNING, "%s in MPEG-1 system streams is not widely supported, " "consider using the vob or the dvd muxer " @@ -363,7 +364,10 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx) stream->id = ac3_id++; } else if (st->codecpar->codec_id == AV_CODEC_ID_DTS) { stream->id = dts_id++; - } else if (st->codecpar->codec_id == AV_CODEC_ID_PCM_S16BE) { + } else if (st->codecpar->codec_id == AV_CODEC_ID_PCM_S16BE || + st->codecpar->codec_id == AV_CODEC_ID_PCM_DVD) { + if (st->codecpar->codec_id == AV_CODEC_ID_PCM_DVD) + av_log(ctx, AV_LOG_WARNING, "Only 16-bit LPCM streams are supported.\n"); stream->id = lpcm_id++; for (j = 0; j < 4; j++) { if (lpcm_freq_tab[j] == st->codecpar->sample_rate) @@ -1150,8 +1154,17 @@ static int mpeg_mux_write_packet(AVFormatContext *ctx, AVPacket *pkt) return AVERROR(ENOMEM); pkt_desc->pts = pts; pkt_desc->dts = dts; + + if (st->codecpar->codec_id == AV_CODEC_ID_PCM_DVD) { + /* Skip first 3 bytes of packet data, which comprise PCM header + and will be written fresh by this muxer. */ + buf += 3; + size -= 3; + } + pkt_desc->unwritten_size = pkt_desc->size = size; + if (!stream->predecode_packet) stream->predecode_packet = pkt_desc; stream->next_packet = &pkt_desc->next;