[Cin] FFmpeg 4.3 mpegts patch - questions ....
Andrew Randrianasulu
randrianasulu at gmail.com
Tue Jul 14 06:40:26 CEST 2020
В сообщении от Monday 13 July 2020 21:42:21 Marton Balint написал(а):
>
> On Mon, 13 Jul 2020, Andrew Randrianasulu wrote:
>
> >
> > src on cgit:
> >
> > https://git.cinelerra-gg.org/git/?p=goodguy/cinelerra.git;a=blob;f=cinelerra-5.1/thirdparty/src/ffmpeg-4.3.patch2;h=81cae5e66d85d3c089603a5a6425cd165aa9f16e;hb=HEAD
> >
> > Well, I don't understand it, just worried about GROWING diff between
> > original ffmpeg and Cin's version of it in this area.
> >
> > (added Marton as CC, as current maintainer of this area of code - sorry for my cont. ignorance, Marton!)
>
> This patch was a bit hard to go through even in its original form, but
> some features got intergrated into ffmpeg master or my ffmpeg m2ts
> branch. I'd at least consider dropping this and using the top 5 patches
> from this branch:
>
> https://github.com/cus/ffmpeg/commits/mpegts2
Thanks for updating this patchest. Unfortunately, I don't have any hardware (BD burner, player) to
play with those accurately enough (software players can be convince to play stream anyway).
>
> However, if somebody wants to mux compliant BluRay i'd probably say
> tsMuxer is still slightly better for the job then ffmpeg, because some
> codec descriptors are still not generated which should be needed for
> BluRay, I don't remember which ones.
At some point in the future I might try to burn (with tsMuxer's help)
"AVC on DVD" type of disk just to see if some far away PS3 will really play it
(but then I think I need to keep bitrate in typical DVD range - 8000 kbits/s or so ...)
>
> Regarding pgs muxing, there was an interesting patch series on
> ffmpeg-devel which should be needed to convert between the pgs format of
> mpegts and matroska, that might be of interest for you too:
>
> https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=1163
Thanks, interesting (I wonder what kind of sw was creating those mkvs,
but probably something proprietary/freeware from doom9 archives :} )
>
> Regards,
> Marton
>
>
>
> >
> > @@ -76,11 +77,12 @@
> > MpegTSSection pat; /* MPEG-2 PAT table */
> > MpegTSSection sdt; /* MPEG-2 SDT table context */
> > MpegTSService **services;
> > - int64_t sdt_period; /* SDT period in PCR time base */
> > - int64_t pat_period; /* PAT/PMT period in PCR time base */
> > + int64_t sdt_packet_timer, sdt_packet_period;
> > + int64_t pat_packet_timer, pat_packet_period;
> > int nb_services;
> > - int64_t first_pcr;
> > - int64_t next_pcr;
> > + int onid;
> > + int tsid;
> >
> > as far as I understand those (onid, tsid) used donwstream:
> >
> > @@ -259,7 +257,7 @@
> > put16(&q, service->sid);
> > put16(&q, 0xe000 | service->pmt.pid);
> > }
> > - mpegts_write_section1(&ts->pat, PAT_TID, ts->transport_stream_id, ts->tables_version, 0, 0,
> > + mpegts_write_section1(&ts->pat, PAT_TID, ts->tsid, ts->tables_version, 0, 0,
> >
> >
> > was it just for shortening typing time?
> >
> > @@ -90,14 +92,14 @@
> > int service_type;
> >
> > int pmt_start_pid;
> > + int pcr_start_pid;
> > int start_pid;
> > int m2ts_mode;
> > - int m2ts_video_pid;
> > - int m2ts_audio_pid;
> > - int m2ts_pgssub_pid;
> > - int m2ts_textsub_pid;
> > + int64_t ts_offset;
> >
> >
> > Hm, it think major point in 4.3 was exactly
> >
> > - Support for muxing pcm and pgs in m2ts
> >
> > ? (PGS are Blu-Ray subtitle graphics - no encoder in ffmpeg yet. But probably you can remux the from existing track?)
> >
> >
> > @@ -217,14 +217,15 @@
> > /* mpegts writer */
> >
> > #define DEFAULT_PROVIDER_NAME "FFmpeg"
> > -#define DEFAULT_SERVICE_NAME "Service"
> > +#define DEFAULT_SERVICE_NAME "Service01"
> >
> >
> > Was it required from your Blu-Ray Disk player? May be it can be made
> > conditional on new m2ts_b (say) mode?
> >
> >
> > -/* we retransmit the SI info at this rate */
> > +/* we retransmit the SI info at this rate (ms) */
> > #define SDT_RETRANS_TIME 500
> > #define PAT_RETRANS_TIME 100
> > -#define PCR_RETRANS_TIME 20
> > +#define PCR_RETRANS_TIME 50
> >
> > may be comment improvement can be made into standalone patch?
> >
> > @@ -281,148 +279,6 @@
> > *q_ptr = q;
> > }
> >
> > -static int get_dvb_stream_type(AVFormatContext *s, AVStream *st)
> > [..]
> > -static int get_m2ts_stream_type(AVFormatContext *s, AVStream *st)
> >
> > here whole two functions are gone ..
> >
> > and probably replaced by homegrown one a bit downstream:
> >
> > @@ -472,8 +320,72 @@
> > err = 1;
> > break;
> > }
> > -
> > - stream_type = ts->m2ts_mode ? get_m2ts_stream_type(s, st) : get_dvb_stream_type(s, st);
> > + switch (st->codecpar->codec_id) {
> >
> >
> > any reason for this move?
> >
> > @@ -736,7 +648,7 @@
> > int i, running_status, free_ca_mode, val;
> >
> > q = data;
> > - put16(&q, ts->original_network_id);
> > + put16(&q, ts->onid);
> >
> >
> > again, just shorter to type?
> >
> > @@ -802,49 +714,12 @@
> > return 0;
> > }
> >
> > -static int64_t get_pcr(const MpegTSWrite *ts, AVIOContext *pb)
> >
> > -static void write_packet(AVFormatContext *s, const uint8_t *packet)
> >
> > -static void section_write_packet(MpegTSSection *s, const uint8_t *packet)
> >
> > all three gone .... can they stay ? :}
> > (for reducing diff size)
> >
> > static MpegTSService *mpegts_add_service(AVFormatContext *s, int sid,
> > - const AVDictionary *metadata,
> > - AVProgram *program)
> > + const char *provider_name,
> > + const char *name)
> > {
> > MpegTSWrite *ts = s->priv_data;
> > MpegTSService *service;
> > - AVDictionaryEntry *title, *provider;
> > - char default_service_name[32];
> > - const char *service_name;
> > - const char *provider_name;
> > -
> > - title = av_dict_get(metadata, "service_name", NULL, 0);
> > - if (!title)
> > - title = av_dict_get(metadata, "title", NULL, 0);
> > - snprintf(default_service_name, sizeof(default_service_name), "%s%02d", DEFAULT_SERVICE_NAME, ts->nb_services + 1);
> > - service_name = title ? title->value : default_service_name;
> > - provider = av_dict_get(metadata, "service_provider", NULL, 0);
> > - provider_name = provider ? provider->value : DEFAULT_PROVIDER_NAME;
> >
> >
> > I think this one allowed more data passing from ...upper libavcodec layer?
> >
> > -static void enable_pcr_generation_for_stream(AVFormatContext *s, AVStream *pcr_st)
> > +static void mpegts_prefix_m2ts_header(AVFormatContext *s)
> > {
> > MpegTSWrite *ts = s->priv_data;
> > - MpegTSWriteStream *ts_st = pcr_st->priv_data;
> > -
> > - if (ts->mux_rate > 1 || ts->pcr_period_ms >= 0) {
> > - int pcr_period_ms = ts->pcr_period_ms == -1 ? PCR_RETRANS_TIME : ts->pcr_period_ms;
> > - ts_st->pcr_period = av_rescale(pcr_period_ms, PCR_TIME_BASE, 1000);
> > - } else {
> > - /* By default, for VBR we select the highest multiple of frame duration which is less than 100 ms. */
> > - int64_t frame_period = 0;
> > - if (pcr_st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
> > - int frame_size = av_get_audio_frame_duration2(pcr_st->codecpar, 0);
> > - if (!frame_size) {
> > - av_log(s, AV_LOG_WARNING, "frame size not set\n");
> > - frame_size = 512;
> > - }
> > - frame_period = av_rescale_rnd(frame_size, PCR_TIME_BASE, pcr_st->codecpar->sample_rate, AV_ROUND_UP);
> > - } else if (pcr_st->avg_frame_rate.num) {
> > - frame_period = av_rescale_rnd(pcr_st->avg_frame_rate.den, PCR_TIME_BASE, pcr_st->avg_frame_rate.num, AV_ROUND_UP);
> > - }
> > - if (frame_period > 0 && frame_period <= PCR_TIME_BASE / 10)
> > - ts_st->pcr_period = frame_period * (PCR_TIME_BASE / 10 / frame_period);
> > - else
> > - ts_st->pcr_period = 1;
> > + if (ts->m2ts_mode) {
> > + uint32_t tp_extra_header = ts->pcr % 0x3fffffff;
> > + tp_extra_header = AV_RB32(&tp_extra_header);
> > + avio_write(s->pb, (unsigned char *) &tp_extra_header,
> > + sizeof(tp_extra_header));
> > }
> > -
> > - // output a PCR as soon as possible
> > - ts_st->last_pcr = ts->first_pcr - ts_st->pcr_period;
> > }
> >
> > May be just leave old function the same, just add short new one
> > (static void mpegts_prefix_m2ts_header(AVFormatContext *s)) ?
> >
> > same for
> >
> > -static void select_pcr_streams(AVFormatContext *s)
> > +static void section_write_packet(MpegTSSection *s, const uint8_t *packet)
> >
> > static int mpegts_init(AVFormatContext *s)
> >
> > ah, so av_dict_get and co moved here ....
> >
> > - /* MPEG pid values < 16 are reserved. Applications which set st->id in
> > - * this range are assigned a calculated pid. */
> > - if (st->id < 16) {
> > - if (ts->m2ts_mode) {
> > - switch (st->codecpar->codec_type) {
> > - case AVMEDIA_TYPE_VIDEO:
> > - ts_st->pid = ts->m2ts_video_pid++;
> > + program = av_find_program_from_stream(s, NULL, i);
> > + if (program) {
> > + for (j = 0; j < ts->nb_services; j++) {
> > + if (ts->services[j]->program == program) {
> > + service = ts->services[j];
> > break;
> > - case AVMEDIA_TYPE_AUDIO:
> > - ts_st->pid = ts->m2ts_audio_pid++;
> > - break;
> > - case AVMEDIA_TYPE_SUBTITLE:
> > - switch (st->codecpar->codec_id) {
> > - case AV_CODEC_ID_HDMV_PGS_SUBTITLE:
> > - ts_st->pid = ts->m2ts_pgssub_pid++;
> > - break;
> > - case AV_CODEC_ID_HDMV_TEXT_SUBTITLE:
> > - ts_st->pid = ts->m2ts_textsub_pid++;
> > - break;
> > - }
> > - break;
> > - }
> > - if (ts->m2ts_video_pid > M2TS_VIDEO_PID + 1 ||
> > - ts->m2ts_audio_pid > M2TS_AUDIO_START_PID + 32 ||
> > - ts->m2ts_pgssub_pid > M2TS_PGSSUB_START_PID + 32 ||
> > - ts->m2ts_textsub_pid > M2TS_TEXTSUB_PID + 1 ||
> > - ts_st->pid < 16) {
> > - av_log(s, AV_LOG_ERROR, "Cannot automatically assign PID for stream %d\n", st->index);
> > - return AVERROR(EINVAL);
> > }
> > - } else {
> > - ts_st->pid = ts->start_pid + i;
> > }
> > - } else {
> > - ts_st->pid = st->id;
> > }
> >
> > Oh, here our subtitle support go .......
> >
> > a lot of skipped, I'm very unsure about all those changes ...
> >
> >
> > @@ -1724,7 +1585,7 @@
> >
> > ret = avio_open_dyn_buf(&ts_st->amux->pb);
> > if (ret < 0)
> > - return ret;
> > + return AVERROR(ENOMEM);
> >
> > seems like possible standalone patch for ffmpeg upstream inclusion?
> >
> > @@ -1755,7 +1616,7 @@
> > } while (p < buf_end && (state & 0x7e) != 2*35 &&
> > (state & 0x7e) >= 2*32);
> >
> > - if ((state & 0x7e) < 2*16 || (state & 0x7e) >= 2*24)
> > + if ((state & 0x7e) < 2*16 && (state & 0x7e) >= 2*24)
> >
> > ???? why it was changed?
> >
> >
> >
> > IIRC those changes come from fact ffmpeg's
> > mpegts muxer mostly geared towards transmission vs standalone BD creation ....
> >
> > But at least potential for BD text subtitles look interestinge nough
> > for keeping looking into this area ...I think.
> >
> > Also, ffmpeg 4.3.1 was already released, with small fixes:
> >
> > https://ffmpeg.org/download.html
> >
> > FFmpeg 4.3.1 "4:3"
> > 4.3.1 was released on 2020-07-11. It is the latest stable FFmpeg release from the 4.3 release branch,
> > which was cut from master on 2020-06-08.
> >
> >
> > It was FAST ......
> >
> > Also, there is one more patch on ML:
> > http://ffmpeg.org/pipermail/ffmpeg-devel/2020-July/266181.html
> >
> > [FFmpeg-devel] [PATCH] x86/yuv2rgb: fix crashes when storing data on unaligned buffers
> >
> > Regression since fc6a5883d6af8cae0e96af84dda0ad74b360a084 on SSSE3 enabled
> > CPUs.
> >
> > Fixes ticket #8747
> >
> > -----
> >
> > END!
> >
>
More information about the Cin
mailing list