[Cin] FFmpeg 4.3 mpegts patch - questions ....
Andrew Randrianasulu
randrianasulu at gmail.com
Mon Jul 13 20:42:27 CEST 2020
В сообщении от Monday 13 July 2020 21:19:20 Good Guy via Cin написал(а):
> Kind of you to take this level of interest,
>
> Well, I don't understand it, just worried about GROWING diff between
> > original ffmpeg and Cin's version of it in this area.
> >
>
> Me too... The purpose of this patch is to make a samsung (model UBD-K8500)
> dvd player accept
> the resulting bluray data from the create_bd operation. There are 3 main
> differences the patch
> attempts to introduce to the default operation.
> 1) suppress sdt output.
> 2) refactor pmt,pat packet scheduling.
> 3) add audio packet priority bit.
> The commercial dvd player seems to be sort of actually damaged in that it
> will fail
> in a variety of bizarre ways if you stray even slightly from a design that
> seems to
> belong to the priesthood of bluray specs, unpublished and spoken only in
> prayer.
>
> I did try the current implementation, with a small tweak to suppress the
> sdt output.
> This attempt was to use the new packet design scheduling and I did notice
> that the audio priority bit is now being applied.
> libaformat/mpegtsenc.c:1345
> if (ts->m2ts_mode && st->codecpar->codec_id == AV_CODEC_ID_AC3)
> val |= 0x20;
> This attempt failed, as the dvd player only played media once, and skipped
> a bunch of audio
> at the beginning. Once it stopped before reaching the end of media. The
> unit has to be
> power cycled before another experiment can be tried. It is not clear what
> the issues are.
> The original 5.1 mod still works, and the new mod is backed out until it
> can be repaired.
> I use this equipment, and I think it is representative of the devices in
> the real world.
>
> It is difficult to even find bluray media that is un-encrypted that can be
> played on this
> commercial player. The un-encrypted media can serve as a template to
> create bd media.
> https://download.blender.org/durian/movies/
>
> The current ffmpeg mpegtsenc implementation may be more aimed at broadcast
> data and not bd dvd system streams, but it is not clear to me just exactly
> what it is
> that the current encoder targets. I have never had good luck with sending
> mods to
> just about anyone. Mostly, they scoff and reject the mods with prejudice.
>
>
> as far as I understand those (onid, tsid) used donwstream:
> > was it just for shortening typing time?
> > 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?)
> >
>
> Those mods are mostly to make the pat,pmt,std presentation match the
> order/timing
> in the sintel example. It is not clear why these are important. It has
> been my experience
> that many commercial drivers are pretty sad implementations designed for
> first to market
> gains, and frequently are quite touchy. If the packets are not processed
> by a state machine,
> then the exact order becomes very important. I am not sure if this is the
> case here, but it is
> clear that the device is very touchy, and gets confused and stays confused
> fast.
>
> This code:
>
> > } while (p < buf_end && (state & 0x1f) != 9 &&
> > (state & 0x1f) != 5 && (state & 0x1f) != 1);
> >
> specializes h264 data and seems to recondition the data. Probably not a
> good idea.
> The encoder should supply the data to the muxer in good to go condition.
>
> If the ffmpeg implementation worked on my bd player, I definitely would use
> it.
> Carrying mods is a pain in the butt.
I also looked at tsMuxer's bugs on GitHub:
https://github.com/justdan96/tsMuxer/issues/108
[Bug] Incorrect TS buffers management #108
Here in comments one of testers complained about tsMuxer's unability to send valid stream
to _some_ devices, while ffmpeg was a success:
-----quote----
Nadahar commented on 11 Feb •
We're using tsMuxer (I've been used to write tsMuxeR - is that "deprecated" now?) to mux video
for a UPnP AV/DLNA server, and have had "strange experiences" with the output for years.
The "clients", called renderers, are often "hardware devices" like TVs, Blu-ray players or game consoles.
While many renderers happily accept tsMuxer's output, some flat our reject them (claiming "invalid"/"corrupt" stream or similar).
This "feels" like a deviation from the standard that many implementations don't care about while some
do, and this issue seems like a perfect candidate for an explanation of the phenomena.
It should be mentioned that while FFmpeg's MPEG-TS output also has issues,
especially with timing information if I remember correctly, its output is generally accepted as
"valid" by renderers. So, despite imperfections in FFmpeg's implementation, they seem to have
gotten something right with regards to "compatibility". I guess it could also be that their output is widespread,
so that decoder implementations are likely to be tested with FFmpeg's output.
I just discovered that tsMuxer had been open-sourced +1 right now, so we have been
using the 2014 version exclusively. That means that this might be a red herring and that whatever
issue caused some renderers to choke has already been addressed by other fixes. This issue just
sounds like a "perfect" explanation.
---quote end----
Unfortunately, it was not disclosed if some suggestions helped or not
https://github.com/justdan96/tsMuxer/issues/141
[Bug] BD Transport stream bitrate is not limited #141
I hope this one can be workarounded by just keeping traget bitrate for elementary streams a bit lower?
Sorry for somewhat pushing this software on you, just as it was noted in comments currently only few
open-source implementations of ts muxers exist, and this one actually was compile-able on my machine (w/o QT GUI):
./tsmuxer
tsMuxeR version git-97d49e4. github.com/justdan96/tsMuxer
tsMuxeR is a simple program to mux video to TS/M2TS files or create BD disks.
tsMuxeR does not use external filters (codecs).
[...]
--blu-ray Mux as a BD disc. If the output file name is a folder, a
Blu-Ray folder structure is created inside that folder.
SSIF files for BD3D discs are not created in this case. If
the output name has an .iso extension, then the disc is
created directly as an image file.
--blu-ray-v3 As above - except mux to UHD BD discs.
--avchd Mux to AVCHD disc.
(last one was said to work with PlayStation *3*)
So, _in theory_, if your device accept stream from it - you can try to abuse it
for various needs - like, muxing subtitles :}
>
> gg
>
>
> On Sun, Jul 12, 2020 at 11:41 PM Andrew Randrianasulu via Cin <
> cin at lists.cinelerra-gg.org> 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!)
> >
> > @@ -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!
> > --
> > Cin mailing list
> > Cin at lists.cinelerra-gg.org
> > https://lists.cinelerra-gg.org/mailman/listinfo/cin
> >
>
More information about the Cin
mailing list