Re: [FFmpeg-devel] [PATCH] mpeg12enc: Use Closed Captions if available
On 08-02-2019 03:18 AM, Carl Eugen Hoyos wrote: 2019-02-07 22:15 GMT+01:00, Gyan : I don't see other h264 decoders, as used within ffmpeg, exporting CC side-data, e.g. OpenH264 or QSV. Do they provide it? QSV offers a method to retrieve it but our wrapper doesn't. (I do see native hevc doing so, as well as the Decklink indev , but not mentioned here.) Seems like a good reason to remove the useless information. Just means a bit more diligence needed before writing docs, not skipping it. Gyan ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg12enc: Use Closed Captions if available
On 2/7/19 10:48 PM, Carl Eugen Hoyos wrote: > Seems like a good reason to remove the useless information. Done in my latest patch for what it's worth, I also don't think it's very important information, plus CC can also be provided by the motivated user through the new_side_data() API anyway :) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg12enc: Use Closed Captions if available
2019-02-07 22:15 GMT+01:00, Gyan : > I don't see other h264 decoders, as used within ffmpeg, exporting CC > side-data, e.g. OpenH264 or QSV. Do they provide it? > (I do see native hevc doing so, as well as the Decklink indev , but not > mentioned here.) Seems like a good reason to remove the useless information. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg12enc: Use Closed Captions if available
On 08-02-2019 01:39 AM, Carl Eugen Hoyos wrote:
2019-02-07 21:07 GMT+01:00, Gyan :
On 08-02-2019 01:18 AM, Carl Eugen Hoyos wrote:
2019-02-07 20:16 GMT+01:00, Mathieu Duponchelle :
---
doc/encoders.texi | 3 +++
libavcodec/mpeg12enc.c | 31 +++
libavcodec/mpegvideo.h | 2 ++
3 files changed, 36 insertions(+)
diff --git a/doc/encoders.texi b/doc/encoders.texi
index e86ae69cc5..378a2ca8eb 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.
+Only the mpeg2 and h264 decoders provide these.
Sorry for the late comment:
This is not a helpful sentence imo, many features are not provided
by all parts of FFmpeg and it is (too) difficult to keep such lists
up-to-date.
Which other decoder are you thinking about?
Let it remain. It is informative
How is it informative?
Which other decoders are you thinking about?
I don't see other h264 decoders, as used within ffmpeg, exporting CC
side-data, e.g. OpenH264 or QSV.
(I do see native hevc doing so, as well as the Decklink indev , but not
mentioned here.)
Full coverage is difficult, but that shouldn't be used as a reason to
suppress any coverage.
Gyan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg12enc: Use Closed Captions if available
2019-02-07 21:07 GMT+01:00, Gyan :
>
>
> On 08-02-2019 01:18 AM, Carl Eugen Hoyos wrote:
>> 2019-02-07 20:16 GMT+01:00, Mathieu Duponchelle :
>>> ---
>>> doc/encoders.texi | 3 +++
>>> libavcodec/mpeg12enc.c | 31 +++
>>> libavcodec/mpegvideo.h | 2 ++
>>> 3 files changed, 36 insertions(+)
>>>
>>> diff --git a/doc/encoders.texi b/doc/encoders.texi
>>> index e86ae69cc5..378a2ca8eb 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.
>>> +Only the mpeg2 and h264 decoders provide these.
>> Sorry for the late comment:
>> This is not a helpful sentence imo, many features are not provided
>> by all parts of FFmpeg and it is (too) difficult to keep such lists
>> up-to-date.
>> Which other decoder are you thinking about?
>>
> Let it remain. It is informative
How is it informative?
Which other decoders are you thinking about?
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg12enc: Use Closed Captions if available
On 08-02-2019 01:18 AM, Carl Eugen Hoyos wrote:
2019-02-07 20:16 GMT+01:00, Mathieu Duponchelle :
---
doc/encoders.texi | 3 +++
libavcodec/mpeg12enc.c | 31 +++
libavcodec/mpegvideo.h | 2 ++
3 files changed, 36 insertions(+)
diff --git a/doc/encoders.texi b/doc/encoders.texi
index e86ae69cc5..378a2ca8eb 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.
+Only the mpeg2 and h264 decoders provide these.
Sorry for the late comment:
This is not a helpful sentence imo, many features are not provided
by all parts of FFmpeg and it is (too) difficult to keep such lists up-to-date.
Which other decoder are you thinking about?
Let it remain. It is informative, and an extremely short list. Can be
updated as needed.
Gyan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg12enc: Use Closed Captions if available
2019-02-07 20:16 GMT+01:00, Mathieu Duponchelle :
> ---
> doc/encoders.texi | 3 +++
> libavcodec/mpeg12enc.c | 31 +++
> libavcodec/mpegvideo.h | 2 ++
> 3 files changed, 36 insertions(+)
>
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index e86ae69cc5..378a2ca8eb 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.
> +Only the mpeg2 and h264 decoders provide these.
Sorry for the late comment:
This is not a helpful sentence imo, many features are not provided
by all parts of FFmpeg and it is (too) difficult to keep such lists up-to-date.
Which other decoder are you thinking about?
> Default is 1 (on).
> @end table
>
> @section png
> diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c
> index d0b458e34b..0e2ab6da47 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,35 @@ 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) {
> +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
If you decide to send another version, please remove the
superfluous brackets.
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg12enc: Use Closed Captions if available
> On Feb 7, 2019, at 1:22 PM, Mathieu Duponchelle > wrote: > > > > On 2/7/19 7:21 PM, Devin Heitmueller wrote: >> Isn’t this calculation incorrect? The max cc_count possible is 31 (0x1F), >> hence the max size should be 93. >> > > True that, updating Not to nitpick, but it might also be worthwhile to create some #define such as MAX_CC_COUNT and have the comparison be "MAX_CC_COUNT * 3”. That makes clear where the magic value “91” came from, and the compiler will optimize out the multiply anyway since it’s a constant. Devin --- Devin Heitmueller - LTN Global Communications dheitmuel...@ltnglobal.com > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg12enc: Use Closed Captions if available
On 2/7/19 7:21 PM, Devin Heitmueller wrote: > Isn’t this calculation incorrect? The max cc_count possible is 31 (0x1F), > hence the max size should be 93. > True that, updating ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg12enc: Use Closed Captions if available
> On Feb 7, 2019, at 1:11 PM, Mathieu Duponchelle
> wrote:
> +if (side_data->size <= 96) {
Isn’t this calculation incorrect? The max cc_count possible is 31 (0x1F),
hence the max size should be 93.
> +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) & 0x1f) | 0x40); // flags,
> cc_count
> +put_bits(>pb, 8, 0xff); // em_data
---
Devin Heitmueller - LTN Global Communications
dheitmuel...@ltnglobal.com
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg12enc: Use Closed Captions if available
On 2/7/19 6:28 PM, Carl Eugen Hoyos wrote: > Should you check here if the size is not bigger than a certain maximum > value ... > >> +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) & 0x1f) | 0x40); // flags, cc_count > ... because of this calculation? Indeed, fixed, attached the updated patch, I'm afraid I'm not entirely familiar with git send-email and the update was posted as an answer to the original post :) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpeg12enc: Use Closed Captions if available
2019-02-07 17:08 GMT+01:00, Mathieu Duponchelle :
> ---
> doc/encoders.texi | 3 +++
> libavcodec/mpeg12enc.c | 24
> libavcodec/mpegvideo.h | 2 ++
> 3 files changed, 29 insertions(+)
>
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index e86ae69cc5..378a2ca8eb 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.
> +Only the mpeg2 and h264 decoders provide these. Default is 1 (on).
> @end table
>
> @section png
> diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c
> index d0b458e34b..7cbb5d652f 100644
> --- a/libavcodec/mpeg12enc.c
> +++ b/libavcodec/mpeg12enc.c
> @@ -544,6 +544,30 @@ 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) {
Should you check here if the size is not bigger than a certain maximum
value ...
> +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) & 0x1f) | 0x40); // flags, cc_count
... because of this calculation?
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
