Re: [FFmpeg-devel] [PATCH v7] mpeg12enc: Use Closed Captions if available

[Mathieu Duponchelle]

Yay, thanks!

On 3/16/19 12:04 AM, Michael Niedermayer wrote:
> no
> will apply
>
> thx
>
> [...]
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v7] mpeg12enc: Use Closed Captions if available

[Michael Niedermayer]

On Thu, Mar 14, 2019 at 09:29:49PM +0100, Mathieu Duponchelle wrote:
> Hello, is there anything preventing from merging this patch?

no
will apply

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v7] mpeg12enc: Use Closed Captions if available

[Mathieu Duponchelle]

Hello, is there anything preventing from merging this patch?

On 2/14/19 10:01 PM, Michael Niedermayer wrote:
> On Wed, Feb 13, 2019 at 11:49:21PM +0100, Mathieu Duponchelle wrote:
>> So, should this go in? :)
> if someone checks it against some spec and or tests it against some
> decoders, probably yes, 
> also might be a good idea to bump the micro version of libavcodec
>
> thanks
>
> [...]
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v7] mpeg12enc: Use Closed Captions if available

[Mathieu Duponchelle]

Ping? :)

On 2/17/19 1:23 PM, Mathieu Duponchelle wrote:
>> It would be better to test against a decoder from a unrelated codebase
>> Otherwise its a bit like testing your new language skills by talking with
>> yourself.
> I have just tested with gstreamer's mpegvideoparse element, it detects the 
> closed captions
> as expected:
>
> 0:00:00.602266814 27833   0x810400 DEBUG mpegvideoparse 
> gstmpegvideoparse.c:507:parse_user_data_packet: CEA-708 closed 
> captions, 60 bytes
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v7] mpeg12enc: Use Closed Captions if available

[Mathieu Duponchelle]

> It would be better to test against a decoder from a unrelated codebase
> Otherwise its a bit like testing your new language skills by talking with
> yourself.

I have just tested with gstreamer's mpegvideoparse element, it detects the 
closed captions
as expected:

0:00:00.602266814 27833   0x810400 DEBUG mpegvideoparse 
gstmpegvideoparse.c:507:parse_user_data_packet: CEA-708 closed 
captions, 60 bytes
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v7] mpeg12enc: Use Closed Captions if available

[Devin Heitmueller]

> It would be better to test against a decoder from a unrelated codebase
> Otherwise its a bit like testing your new language skills by talking with
> yourself.

It should be pretty easy to just play the resulting TS in VLC and
confirm the captions are present and play correctly.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v7] mpeg12enc: Use Closed Captions if available

[Michael Niedermayer]

On Fri, Feb 15, 2019 at 11:17:38PM +0100, Mathieu Duponchelle wrote:
> On 2/14/19 10:01 PM, Michael Niedermayer wrote:
> > if someone checks it against some spec and or tests it against some
> > decoders, probably yes,
> 
> The relevant part of the spec is section 6.2.3 in
> ,
> and I have indeed tested this patch against FFmpeg's mpeg2video decoder,
> it seems to work just fine :)

It would be better to test against a decoder from a unrelated codebase
Otherwise its a bit like testing your new language skills by talking with
yourself.


> 
> Note that this commit is very similar in intent to
> e06114fed3afa69187b3dfc09a7a1a25cfd558b3 if that helps.
> 
> >  
> > also might be a good idea to bump the micro version of libavcodec
> 
> I'm not familiar with this procedure I must admit :)

you add 1 to the value after LIBAVCODEC_VERSION_MICRO
of course this could also be done by whoever commits this ...
just saying someone should when this is pushed

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v7] mpeg12enc: Use Closed Captions if available

[Mathieu Duponchelle]

On 2/14/19 10:01 PM, Michael Niedermayer wrote:
> if someone checks it against some spec and or tests it against some
> decoders, probably yes,

The relevant part of the spec is section 6.2.3 in
,
and I have indeed tested this patch against FFmpeg's mpeg2video decoder,
it seems to work just fine :)

Note that this commit is very similar in intent to
e06114fed3afa69187b3dfc09a7a1a25cfd558b3 if that helps.

>  
> also might be a good idea to bump the micro version of libavcodec

I'm not familiar with this procedure I must admit :)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v7] mpeg12enc: Use Closed Captions if available

[Michael Niedermayer]

On Wed, Feb 13, 2019 at 11:49:21PM +0100, Mathieu Duponchelle wrote:
> So, should this go in? :)

if someone checks it against some spec and or tests it against some
decoders, probably yes, 
also might be a good idea to bump the micro version of libavcodec

thanks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v7] mpeg12enc: Use Closed Captions if available

[Mathieu Duponchelle]

So, should this go in? :)

On 2/8/19 11:10 PM, Mathieu Duponchelle wrote:
> ---
>  doc/encoders.texi  |  3 +++
>  libavcodec/mpeg12enc.c | 32 
>  libavcodec/mpegvideo.h |  2 ++
>  3 files changed, 37 insertions(+)
>
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index e86ae69cc5..a283b9fddf 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -2574,6 +2574,9 @@ Specifies the video_format written into the sequence 
> display extension
>  indicating the source of the video pictures. The default is 
> @samp{unspecified},
>  can be @samp{component}, @samp{pal}, @samp{ntsc}, @samp{secam} or @samp{mac}.
>  For maximum compatibility, use @samp{component}.
> +@item a53cc @var{boolean}
> +Import closed captions (which must be ATSC compatible format) into output.
> +Default is 1 (on).
>  @end table
>  
>  @section png
> diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c
> index d0b458e34b..2bc5289d63 100644
> --- a/libavcodec/mpeg12enc.c
> +++ b/libavcodec/mpeg12enc.c
> @@ -61,6 +61,8 @@ static uint32_t mpeg1_chr_dc_uni[512];
>  static uint8_t mpeg1_index_run[2][64];
>  static int8_t  mpeg1_max_level[2][64];
>  
> +#define A53_MAX_CC_COUNT 0x1f
> +
>  static av_cold void init_uni_ac_vlc(RLTable *rl, uint8_t *uni_ac_vlc_len)
>  {
>  int i;
> @@ -544,6 +546,36 @@ void ff_mpeg1_encode_picture_header(MpegEncContext *s, 
> int picture_number)
>  }
>  }
>  
> +if (s->codec_id == AV_CODEC_ID_MPEG2VIDEO && s->a53_cc) {
> +side_data = av_frame_get_side_data(s->current_picture_ptr->f,
> +AV_FRAME_DATA_A53_CC);
> +if (side_data) {
> +if (side_data->size <= A53_MAX_CC_COUNT * 3 && side_data->size % 
> 3 == 0) {
> +int i = 0;
> +
> +put_header (s, USER_START_CODE);
> +
> +put_bits(>pb, 8, 'G');   // 
> user_identifier
> +put_bits(>pb, 8, 'A');
> +put_bits(>pb, 8, '9');
> +put_bits(>pb, 8, '4');
> +put_bits(>pb, 8, 3); // 
> user_data_type_code
> +put_bits(>pb, 8,
> +(side_data->size / 3 & A53_MAX_CC_COUNT) | 0x40); // 
> flags, cc_count
> +put_bits(>pb, 8, 0xff);  // em_data
> +
> +for (i = 0; i < side_data->size; i++)
> +put_bits(>pb, 8, side_data->data[i]);
> +
> +put_bits(>pb, 8, 0xff);  // marker_bits
> +} else {
> +av_log(s->avctx, AV_LOG_WARNING,
> +"Warning Closed Caption size (%d) can not exceed 93 
> bytes "
> +"and must be a multiple of 3\n", side_data->size);
> +}
> +}
> +}
> +
>  s->mb_y = 0;
>  ff_mpeg1_encode_slice_header(s);
>  }
> diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h
> index bbc6b5646a..e1ff5f97dc 100644
> --- a/libavcodec/mpegvideo.h
> +++ b/libavcodec/mpegvideo.h
> @@ -455,6 +455,7 @@ typedef struct MpegEncContext {
>  /* MPEG-2-specific - I wished not to have to support this mess. */
>  int progressive_sequence;
>  int mpeg_f_code[2][2];
> +int a53_cc;
>  
>  // picture structure defines are loaded from mpegutils.h
>  int picture_structure;
> @@ -663,6 +664,7 @@ FF_MPV_OPT_CMP_FUNC, \
>  {"ps", "RTP payload size in bytes", 
> FF_MPV_OFFSET(rtp_payload_size), AV_OPT_TYPE_INT, {.i64 = 0 }, INT_MIN, 
> INT_MAX, FF_MPV_OPT_FLAGS }, \
>  {"mepc", "Motion estimation bitrate penalty compensation (1.0 = 256)", 
> FF_MPV_OFFSET(me_penalty_compensation), AV_OPT_TYPE_INT, {.i64 = 256 }, 
> INT_MIN, INT_MAX, FF_MPV_OPT_FLAGS }, \
>  {"mepre", "pre motion estimation", FF_MPV_OFFSET(me_pre), AV_OPT_TYPE_INT, 
> {.i64 = 0 }, INT_MIN, INT_MAX, FF_MPV_OPT_FLAGS }, \
> +{"a53cc", "Use A53 Closed Captions (if available)", FF_MPV_OFFSET(a53_cc), 
> AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, FF_MPV_OPT_FLAGS }, \
>  
>  extern const AVOption ff_mpv_generic_options[];
>  
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH v7] mpeg12enc: Use Closed Captions if available

[Mathieu Duponchelle]

---
 doc/encoders.texi  |  3 +++
 libavcodec/mpeg12enc.c | 32 
 libavcodec/mpegvideo.h |  2 ++
 3 files changed, 37 insertions(+)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index e86ae69cc5..a283b9fddf 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -2574,6 +2574,9 @@ Specifies the video_format written into the sequence 
display extension
 indicating the source of the video pictures. The default is @samp{unspecified},
 can be @samp{component}, @samp{pal}, @samp{ntsc}, @samp{secam} or @samp{mac}.
 For maximum compatibility, use @samp{component}.
+@item a53cc @var{boolean}
+Import closed captions (which must be ATSC compatible format) into output.
+Default is 1 (on).
 @end table
 
 @section png
diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c
index d0b458e34b..2bc5289d63 100644
--- a/libavcodec/mpeg12enc.c
+++ b/libavcodec/mpeg12enc.c
@@ -61,6 +61,8 @@ static uint32_t mpeg1_chr_dc_uni[512];
 static uint8_t mpeg1_index_run[2][64];
 static int8_t  mpeg1_max_level[2][64];
 
+#define A53_MAX_CC_COUNT 0x1f
+
 static av_cold void init_uni_ac_vlc(RLTable *rl, uint8_t *uni_ac_vlc_len)
 {
 int i;
@@ -544,6 +546,36 @@ void ff_mpeg1_encode_picture_header(MpegEncContext *s, int 
picture_number)
 }
 }
 
+if (s->codec_id == AV_CODEC_ID_MPEG2VIDEO && s->a53_cc) {
+side_data = av_frame_get_side_data(s->current_picture_ptr->f,
+AV_FRAME_DATA_A53_CC);
+if (side_data) {
+if (side_data->size <= A53_MAX_CC_COUNT * 3 && side_data->size % 3 
== 0) {
+int i = 0;
+
+put_header (s, USER_START_CODE);
+
+put_bits(>pb, 8, 'G');   // user_identifier
+put_bits(>pb, 8, 'A');
+put_bits(>pb, 8, '9');
+put_bits(>pb, 8, '4');
+put_bits(>pb, 8, 3); // 
user_data_type_code
+put_bits(>pb, 8,
+(side_data->size / 3 & A53_MAX_CC_COUNT) | 0x40); // 
flags, cc_count
+put_bits(>pb, 8, 0xff);  // em_data
+
+for (i = 0; i < side_data->size; i++)
+put_bits(>pb, 8, side_data->data[i]);
+
+put_bits(>pb, 8, 0xff);  // marker_bits
+} else {
+av_log(s->avctx, AV_LOG_WARNING,
+"Warning Closed Caption size (%d) can not exceed 93 bytes "
+"and must be a multiple of 3\n", side_data->size);
+}
+}
+}
+
 s->mb_y = 0;
 ff_mpeg1_encode_slice_header(s);
 }
diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h
index bbc6b5646a..e1ff5f97dc 100644
--- a/libavcodec/mpegvideo.h
+++ b/libavcodec/mpegvideo.h
@@ -455,6 +455,7 @@ typedef struct MpegEncContext {
 /* MPEG-2-specific - I wished not to have to support this mess. */
 int progressive_sequence;
 int mpeg_f_code[2][2];
+int a53_cc;
 
 // picture structure defines are loaded from mpegutils.h
 int picture_structure;
@@ -663,6 +664,7 @@ FF_MPV_OPT_CMP_FUNC, \
 {"ps", "RTP payload size in bytes", 
FF_MPV_OFFSET(rtp_payload_size), AV_OPT_TYPE_INT, {.i64 = 0 }, INT_MIN, 
INT_MAX, FF_MPV_OPT_FLAGS }, \
 {"mepc", "Motion estimation bitrate penalty compensation (1.0 = 256)", 
FF_MPV_OFFSET(me_penalty_compensation), AV_OPT_TYPE_INT, {.i64 = 256 }, 
INT_MIN, INT_MAX, FF_MPV_OPT_FLAGS }, \
 {"mepre", "pre motion estimation", FF_MPV_OFFSET(me_pre), AV_OPT_TYPE_INT, 
{.i64 = 0 }, INT_MIN, INT_MAX, FF_MPV_OPT_FLAGS }, \
+{"a53cc", "Use A53 Closed Captions (if available)", FF_MPV_OFFSET(a53_cc), 
AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, FF_MPV_OPT_FLAGS }, \
 
 extern const AVOption ff_mpv_generic_options[];
 
-- 
2.20.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel