| Message ID | HE1PR0301MB2154DFEA4856F8C5CF45179C8F799@HE1PR0301MB2154.eurprd03.prod.outlook.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel] avformat/mpegtsenc: Preserve disposition in the absence of language | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
ISO 639 language descriptors used by DVB are the 3 charecter variants hence
const char *language = lang && strlen(lang->value) >= 3 ? lang->value : default_language;should beconst char *language = lang && strlen(lang->value) > 3 ? lang->value : default_language;
On Saturday, 3 April 2021, 06:53:20 BST, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:
Implements ticket #9113.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavformat/mpegtsenc.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index 35c835c484..dbd3bb148a 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -459,6 +459,8 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
AVStream *st = s->streams[i];
MpegTSWriteStream *ts_st = st->priv_data;
AVDictionaryEntry *lang = av_dict_get(st->metadata, "language", NULL, 0);
+ const char default_language[] = "und";
+ const char *language = lang && strlen(lang->value) >= 3 ? lang->value : default_language;
enum AVCodecID codec_id = st->codecpar->codec_id;
if (s->nb_programs) {
@@ -598,16 +600,19 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
}
}
- if (lang) {
- char *p;
- char *next = lang->value;
+ if (language != default_language ||
+ st->disposition & (AV_DISPOSITION_CLEAN_EFFECTS |
+ AV_DISPOSITION_HEARING_IMPAIRED |
+ AV_DISPOSITION_VISUAL_IMPAIRED)) {
+ const char *p;
+ const char *next = language;
uint8_t *len_ptr;
*q++ = ISO_639_LANGUAGE_DESCRIPTOR;
len_ptr = q++;
*len_ptr = 0;
- for (p = lang->value; next && *len_ptr < 255 / 4 * 4; p = next + 1) {
+ for (p = next; next && *len_ptr < 255 / 4 * 4; p = next + 1) {
if (q - data > SECTION_LENGTH - 4) {
err = 1;
break;
@@ -637,10 +642,6 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
}
break;
case AVMEDIA_TYPE_SUBTITLE:
- {
- const char default_language[] = "und";
- const char *language = lang && strlen(lang->value) >= 3 ? lang->value : default_language;
-
if (codec_id == AV_CODEC_ID_DVB_SUBTITLE) {
uint8_t *len_ptr;
int extradata_copied = 0;
@@ -715,7 +716,6 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
*len_ptr = q - len_ptr - 1;
}
- }
break;
case AVMEDIA_TYPE_VIDEO:
if (stream_type == STREAM_TYPE_VIDEO_DIRAC) {
stephen douglas: > ISO 639 language descriptors used by DVB are the 3 charecter variants hence > const char *language = lang && strlen(lang->value) >= 3 ? lang->value : default_language;should beconst char *language = lang && strlen(lang->value) > 3 ? lang->value : default_language; 1. Stop top-posting. 2. Are you even aware that strlen does not include the trailing \0? > On Saturday, 3 April 2021, 06:53:20 BST, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > > Implements ticket #9113. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > ---
On Sat, 3 Apr 2021, Andreas Rheinhardt wrote: > Implements ticket #9113. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavformat/mpegtsenc.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c > index 35c835c484..dbd3bb148a 100644 > --- a/libavformat/mpegtsenc.c > +++ b/libavformat/mpegtsenc.c > @@ -459,6 +459,8 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service) > AVStream *st = s->streams[i]; > MpegTSWriteStream *ts_st = st->priv_data; > AVDictionaryEntry *lang = av_dict_get(st->metadata, "language", NULL, 0); > + const char default_language[] = "und"; > + const char *language = lang && strlen(lang->value) >= 3 ? lang->value : default_language; > enum AVCodecID codec_id = st->codecpar->codec_id; > > if (s->nb_programs) { > @@ -598,16 +600,19 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service) > } > } > > - if (lang) { > - char *p; > - char *next = lang->value; > + if (language != default_language || > + st->disposition & (AV_DISPOSITION_CLEAN_EFFECTS | > + AV_DISPOSITION_HEARING_IMPAIRED | > + AV_DISPOSITION_VISUAL_IMPAIRED)) { > + const char *p; > + const char *next = language; > uint8_t *len_ptr; > > *q++ = ISO_639_LANGUAGE_DESCRIPTOR; > len_ptr = q++; > *len_ptr = 0; > > - for (p = lang->value; next && *len_ptr < 255 / 4 * 4; p = next + 1) { > + for (p = next; next && *len_ptr < 255 / 4 * 4; p = next + 1) { Maybe it would make the code more readable to do both initializations in the for() statement, e.g.: for (p = next = language; ...) LGTM otherwise. Thanks, Marton > if (q - data > SECTION_LENGTH - 4) { > err = 1; > break; > @@ -637,10 +642,6 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service) > } > break; > case AVMEDIA_TYPE_SUBTITLE: > - { > - const char default_language[] = "und"; > - const char *language = lang && strlen(lang->value) >= 3 ? lang->value : default_language; > - > if (codec_id == AV_CODEC_ID_DVB_SUBTITLE) { > uint8_t *len_ptr; > int extradata_copied = 0; > @@ -715,7 +716,6 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service) > > *len_ptr = q - len_ptr - 1; > } > - } > break; > case AVMEDIA_TYPE_VIDEO: > if (stream_type == STREAM_TYPE_VIDEO_DIRAC) { > -- > 2.27.0 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Marton Balint: > > > On Sat, 3 Apr 2021, Andreas Rheinhardt wrote: > >> Implements ticket #9113. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> libavformat/mpegtsenc.c | 18 +++++++++--------- >> 1 file changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c >> index 35c835c484..dbd3bb148a 100644 >> --- a/libavformat/mpegtsenc.c >> +++ b/libavformat/mpegtsenc.c >> @@ -459,6 +459,8 @@ static int mpegts_write_pmt(AVFormatContext *s, >> MpegTSService *service) >> AVStream *st = s->streams[i]; >> MpegTSWriteStream *ts_st = st->priv_data; >> AVDictionaryEntry *lang = av_dict_get(st->metadata, >> "language", NULL, 0); >> + const char default_language[] = "und"; >> + const char *language = lang && strlen(lang->value) >= 3 ? >> lang->value : default_language; >> enum AVCodecID codec_id = st->codecpar->codec_id; >> >> if (s->nb_programs) { >> @@ -598,16 +600,19 @@ static int mpegts_write_pmt(AVFormatContext *s, >> MpegTSService *service) >> } >> } >> >> - if (lang) { >> - char *p; >> - char *next = lang->value; >> + if (language != default_language || >> + st->disposition & (AV_DISPOSITION_CLEAN_EFFECTS | >> + AV_DISPOSITION_HEARING_IMPAIRED | >> + AV_DISPOSITION_VISUAL_IMPAIRED)) { >> + const char *p; >> + const char *next = language; >> uint8_t *len_ptr; >> >> *q++ = ISO_639_LANGUAGE_DESCRIPTOR; >> len_ptr = q++; >> *len_ptr = 0; >> >> - for (p = lang->value; next && *len_ptr < 255 / 4 * 4; >> p = next + 1) { >> + for (p = next; next && *len_ptr < 255 / 4 * 4; p = >> next + 1) { > > Maybe it would make the code more readable to do both initializations in > the for() statement, e.g.: for (p = next = language; ...) > The reason I didn't do so is that it would make the line above 80 characters. So the alternatives to my current patch would be: for (p = next = language; next && *len_ptr < 255 / 4 * 4; p = next + 1) { or for (const char *p = language, *next = p; next && *len_ptr < 255 / 4 * 4; p = next + 1) { or factoring out writing the ISO 639 language descriptor into a function of its own or just ignoring the 80 chars line limit. What do you prefer? > LGTM otherwise. > > Thanks, > Marton > >> if (q - data > SECTION_LENGTH - 4) { >> err = 1; >> break; >> @@ -637,10 +642,6 @@ static int mpegts_write_pmt(AVFormatContext *s, >> MpegTSService *service) >> } >> break; >> case AVMEDIA_TYPE_SUBTITLE: >> - { >> - const char default_language[] = "und"; >> - const char *language = lang && strlen(lang->value) >= 3 ? >> lang->value : default_language; >> - >> if (codec_id == AV_CODEC_ID_DVB_SUBTITLE) { >> uint8_t *len_ptr; >> int extradata_copied = 0; >> @@ -715,7 +716,6 @@ static int mpegts_write_pmt(AVFormatContext *s, >> MpegTSService *service) >> >> *len_ptr = q - len_ptr - 1; >> } >> - } >> break; >> case AVMEDIA_TYPE_VIDEO: >> if (stream_type == STREAM_TYPE_VIDEO_DIRAC) { >> -- >> 2.27.0 >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Sun, 4 Apr 2021, Andreas Rheinhardt wrote: > Marton Balint: >> >> >> On Sat, 3 Apr 2021, Andreas Rheinhardt wrote: >> >>> Implements ticket #9113. >>> >>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >>> --- >>> libavformat/mpegtsenc.c | 18 +++++++++--------- >>> 1 file changed, 9 insertions(+), 9 deletions(-) >>> >>> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c >>> index 35c835c484..dbd3bb148a 100644 >>> --- a/libavformat/mpegtsenc.c >>> +++ b/libavformat/mpegtsenc.c >>> @@ -459,6 +459,8 @@ static int mpegts_write_pmt(AVFormatContext *s, >>> MpegTSService *service) >>> AVStream *st = s->streams[i]; >>> MpegTSWriteStream *ts_st = st->priv_data; >>> AVDictionaryEntry *lang = av_dict_get(st->metadata, >>> "language", NULL, 0); >>> + const char default_language[] = "und"; >>> + const char *language = lang && strlen(lang->value) >= 3 ? >>> lang->value : default_language; >>> enum AVCodecID codec_id = st->codecpar->codec_id; >>> >>> if (s->nb_programs) { >>> @@ -598,16 +600,19 @@ static int mpegts_write_pmt(AVFormatContext *s, >>> MpegTSService *service) >>> } >>> } >>> >>> - if (lang) { >>> - char *p; >>> - char *next = lang->value; >>> + if (language != default_language || >>> + st->disposition & (AV_DISPOSITION_CLEAN_EFFECTS | >>> + AV_DISPOSITION_HEARING_IMPAIRED | >>> + AV_DISPOSITION_VISUAL_IMPAIRED)) { >>> + const char *p; >>> + const char *next = language; >>> uint8_t *len_ptr; >>> >>> *q++ = ISO_639_LANGUAGE_DESCRIPTOR; >>> len_ptr = q++; >>> *len_ptr = 0; >>> >>> - for (p = lang->value; next && *len_ptr < 255 / 4 * 4; >>> p = next + 1) { >>> + for (p = next; next && *len_ptr < 255 / 4 * 4; p = >>> next + 1) { >> >> Maybe it would make the code more readable to do both initializations in >> the for() statement, e.g.: for (p = next = language; ...) >> > > The reason I didn't do so is that it would make the line above 80 > characters. So the alternatives to my current patch would be: > > for (p = next = language; next && *len_ptr < 255 / 4 * 4; > p = next + 1) { I like this the most wrapped to a single line. I don't think adhereing to the 80 char limit makes the code more readable here, so I'd simply ignore it. Regards, Marton > > or > > for (const char *p = language, *next = p; > next && *len_ptr < 255 / 4 * 4; p = next + 1) { > > or factoring out writing the ISO 639 language descriptor into a function > of its own or just ignoring the 80 chars line limit. > What do you prefer? > >> LGTM otherwise. >> >> Thanks, >> Marton >> >>> if (q - data > SECTION_LENGTH - 4) { >>> err = 1; >>> break; >>> @@ -637,10 +642,6 @@ static int mpegts_write_pmt(AVFormatContext *s, >>> MpegTSService *service) >>> } >>> break; >>> case AVMEDIA_TYPE_SUBTITLE: >>> - { >>> - const char default_language[] = "und"; >>> - const char *language = lang && strlen(lang->value) >= 3 ? >>> lang->value : default_language; >>> - >>> if (codec_id == AV_CODEC_ID_DVB_SUBTITLE) { >>> uint8_t *len_ptr; >>> int extradata_copied = 0; >>> @@ -715,7 +716,6 @@ static int mpegts_write_pmt(AVFormatContext *s, >>> MpegTSService *service) >>> >>> *len_ptr = q - len_ptr - 1; >>> } >>> - } >>> break; >>> case AVMEDIA_TYPE_VIDEO: >>> if (stream_type == STREAM_TYPE_VIDEO_DIRAC) { >>> -- >>> 2.27.0 >>> >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >>> To unsubscribe, visit link above, or email >>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c index 35c835c484..dbd3bb148a 100644 --- a/libavformat/mpegtsenc.c +++ b/libavformat/mpegtsenc.c @@ -459,6 +459,8 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service) AVStream *st = s->streams[i]; MpegTSWriteStream *ts_st = st->priv_data; AVDictionaryEntry *lang = av_dict_get(st->metadata, "language", NULL, 0); + const char default_language[] = "und"; + const char *language = lang && strlen(lang->value) >= 3 ? lang->value : default_language; enum AVCodecID codec_id = st->codecpar->codec_id; if (s->nb_programs) { @@ -598,16 +600,19 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service) } } - if (lang) { - char *p; - char *next = lang->value; + if (language != default_language || + st->disposition & (AV_DISPOSITION_CLEAN_EFFECTS | + AV_DISPOSITION_HEARING_IMPAIRED | + AV_DISPOSITION_VISUAL_IMPAIRED)) { + const char *p; + const char *next = language; uint8_t *len_ptr; *q++ = ISO_639_LANGUAGE_DESCRIPTOR; len_ptr = q++; *len_ptr = 0; - for (p = lang->value; next && *len_ptr < 255 / 4 * 4; p = next + 1) { + for (p = next; next && *len_ptr < 255 / 4 * 4; p = next + 1) { if (q - data > SECTION_LENGTH - 4) { err = 1; break; @@ -637,10 +642,6 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service) } break; case AVMEDIA_TYPE_SUBTITLE: - { - const char default_language[] = "und"; - const char *language = lang && strlen(lang->value) >= 3 ? lang->value : default_language; - if (codec_id == AV_CODEC_ID_DVB_SUBTITLE) { uint8_t *len_ptr; int extradata_copied = 0; @@ -715,7 +716,6 @@ static int mpegts_write_pmt(AVFormatContext *s, MpegTSService *service) *len_ptr = q - len_ptr - 1; } - } break; case AVMEDIA_TYPE_VIDEO: if (stream_type == STREAM_TYPE_VIDEO_DIRAC) {
Implements ticket #9113. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavformat/mpegtsenc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)