[FFmpeg-devel,3/4] avformat/mpegenc - accept PCM_DVD streams

Submitted by Gyan on Feb. 1, 2018, 2:22 p.m.

Details

Message ID bb07ce7c-4ebb-32cb-25d8-da21ff9d3f26@gmail.com
State New
Headers show

Commit Message

Gyan Feb. 1, 2018, 2:22 p.m.
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.
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(-)

Comments

Michael Niedermayer Feb. 2, 2018, 10:29 p.m.
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)

[...]
Michael Niedermayer Feb. 2, 2018, 10:32 p.m.
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
Gyan Feb. 8, 2018, 8:34 a.m.
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
Michael Niedermayer Feb. 8, 2018, 10:09 p.m.
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

[...]

Patch hide | download patch | download mbox

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;