<div dir="ltr"><div>Kind of you to take this level of interest,</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>Well, I don't understand it, just worried about GROWING diff between <br>
original ffmpeg and Cin's version of it in this area.<br></div></blockquote><div> </div><div>Me too... The purpose of this patch is to make a samsung (model UBD-K8500) dvd player accept</div><div>the resulting bluray data from the create_bd operation.  There are 3 main differences the patch</div><div>attempts to introduce to the default operation.</div><div>1) suppress sdt output.</div><div>2) refactor pmt,pat packet scheduling.</div><div>3) add audio packet priority bit.</div><div>The commercial dvd player seems to be sort of actually damaged in that it will fail</div><div>in a variety of bizarre ways if you stray even slightly from a design that seems to</div><div>belong to the priesthood of bluray specs, unpublished and spoken only in prayer.</div><div><br></div><div>I did try the current implementation, with a small tweak to suppress the sdt output.</div><div>This attempt was to use the new packet design scheduling and I did notice</div><div>that the audio priority bit is now being applied.</div>libaformat/mpegtsenc.c:1345<br>        if (ts->m2ts_mode && st->codecpar->codec_id == AV_CODEC_ID_AC3)<br><div>            val |= 0x20;</div><div>This attempt failed, as the dvd player only played media once, and skipped a bunch of audio</div><div>at the beginning.  Once it stopped before reaching the end of media.  The unit has to be</div><div>power cycled before another experiment can be tried.  It is not clear what the issues are.</div><div>The original 5.1 mod still works, and the new mod is backed out until it can be repaired.</div><div>I use this equipment, and I think it is representative of the devices in the real world.</div><div><br></div><div>It is difficult to even find bluray media that is un-encrypted that can be played on this</div><div>commercial player.  The un-encrypted media can serve as a template to create bd media.</div><div><a href="https://download.blender.org/durian/movies/">https://download.blender.org/durian/movies/</a></div><div><br></div><div>The current ffmpeg mpegtsenc implementation may be more aimed at broadcast</div><div>data and not bd dvd system streams, but it is not clear to me just exactly what it is</div><div>that the current encoder targets.  I have never had good luck with sending mods to</div><div>just about anyone.  Mostly, they scoff and reject the mods with prejudice.</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>as far as I understand those  (onid, tsid) used donwstream:</div><div>was it just for shortening typing time?</div><div>Hm, it think major point in 4.3 was exactly <br>
- Support for muxing pcm and pgs in m2ts <br>
? (PGS are Blu-Ray subtitle graphics - no encoder in ffmpeg yet. But probably you can remux the from existing track?)<br></div></blockquote><div><br></div><div>Those mods are mostly to make the pat,pmt,std presentation match the order/timing</div><div> in the sintel example.  It is not clear why these are important.  It has been my experience</div><div>that many commercial drivers are pretty sad implementations designed for first to market</div><div>gains, and frequently are quite touchy.  If the packets are not processed by a state machine,</div><div>then the exact order becomes very important.  I am not sure if this is the case here, but it is</div><div>clear that the device is very touchy, and gets confused and stays confused fast.</div><div><br></div><div>This code:</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>        } while (p < buf_end && (state & 0x1f) != 9 &&<br>                 (state & 0x1f) != 5 && (state & 0x1f) != 1);<br></div></blockquote><div>specializes h264 data and seems to recondition the data.  Probably not a good idea.</div><div>The encoder should supply the data to the muxer in good to go condition.</div><div><br></div><div>If the ffmpeg implementation worked on my bd player, I definitely would use it.</div><div>Carrying mods is a pain in the butt.</div><div><br></div><div>gg</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Jul 12, 2020 at 11:41 PM Andrew Randrianasulu via Cin <<a href="mailto:cin@lists.cinelerra-gg.org">cin@lists.cinelerra-gg.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
src on cgit:<br>
<br>
<a href="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" rel="noreferrer" target="_blank">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</a><br>
<br>
Well, I don't understand it, just worried about GROWING diff between <br>
original ffmpeg and Cin's version of it in this area.<br>
<br>
(added Marton as CC, as current maintainer of this area of code - sorry for my cont. ignorance, Marton!) <br>
<br>
@@ -76,11 +77,12 @@<br>
     MpegTSSection pat; /* MPEG-2 PAT table */<br>
     MpegTSSection sdt; /* MPEG-2 SDT table context */<br>
     MpegTSService **services;<br>
-    int64_t sdt_period; /* SDT period in PCR time base */<br>
-    int64_t pat_period; /* PAT/PMT period in PCR time base */<br>
+    int64_t sdt_packet_timer, sdt_packet_period;<br>
+    int64_t pat_packet_timer, pat_packet_period;<br>
     int nb_services;<br>
-    int64_t first_pcr;<br>
-    int64_t next_pcr;<br>
+    int onid;<br>
+    int tsid;<br>
<br>
as far as I understand those  (onid, tsid) used donwstream:<br>
<br>
@@ -259,7 +257,7 @@<br>
         put16(&q, service->sid);<br>
         put16(&q, 0xe000 | service->pmt.pid);<br>
     }<br>
-    mpegts_write_section1(&ts->pat, PAT_TID, ts->transport_stream_id, ts->tables_version, 0, 0,<br>
+    mpegts_write_section1(&ts->pat, PAT_TID, ts->tsid, ts->tables_version, 0, 0,<br>
<br>
<br>
was it just for shortening typing time?<br>
<br>
@@ -90,14 +92,14 @@<br>
     int service_type;<br>
<br>
     int pmt_start_pid;<br>
+    int pcr_start_pid;<br>
     int start_pid;<br>
     int m2ts_mode;<br>
-    int m2ts_video_pid;<br>
-    int m2ts_audio_pid;<br>
-    int m2ts_pgssub_pid;<br>
-    int m2ts_textsub_pid;<br>
+    int64_t ts_offset;<br>
<br>
<br>
Hm, it think major point in 4.3 was exactly <br>
<br>
- Support for muxing pcm and pgs in m2ts <br>
<br>
? (PGS are Blu-Ray subtitle graphics - no encoder in ffmpeg yet. But probably you can remux the from existing track?)<br>
<br>
<br>
@@ -217,14 +217,15 @@<br>
 /* mpegts writer */<br>
<br>
 #define DEFAULT_PROVIDER_NAME   "FFmpeg"<br>
-#define DEFAULT_SERVICE_NAME    "Service"<br>
+#define DEFAULT_SERVICE_NAME    "Service01"<br>
<br>
<br>
Was it required from your Blu-Ray Disk player? May be it can be made<br>
conditional on new m2ts_b  (say) mode?<br>
<br>
<br>
-/* we retransmit the SI info at this rate */<br>
+/* we retransmit the SI info at this rate (ms) */<br>
 #define SDT_RETRANS_TIME 500<br>
 #define PAT_RETRANS_TIME 100<br>
-#define PCR_RETRANS_TIME 20<br>
+#define PCR_RETRANS_TIME 50<br>
<br>
may be comment improvement can be made into standalone patch?<br>
<br>
@@ -281,148 +279,6 @@<br>
     *q_ptr = q;<br>
 }<br>
<br>
-static int get_dvb_stream_type(AVFormatContext *s, AVStream *st)<br>
[..]<br>
-static int get_m2ts_stream_type(AVFormatContext *s, AVStream *st)<br>
<br>
here whole two functions are gone ..<br>
<br>
and probably replaced by homegrown one a bit downstream:<br>
<br>
@@ -472,8 +320,72 @@<br>
             err = 1;<br>
             break;<br>
         }<br>
-<br>
-        stream_type = ts->m2ts_mode ? get_m2ts_stream_type(s, st) : get_dvb_stream_type(s, st);<br>
+        switch (st->codecpar->codec_id) {<br>
<br>
<br>
any reason for this move?<br>
<br>
@@ -736,7 +648,7 @@<br>
     int i, running_status, free_ca_mode, val;<br>
<br>
     q = data;<br>
-    put16(&q, ts->original_network_id);<br>
+    put16(&q, ts->onid);<br>
<br>
<br>
again, just shorter to type?<br>
<br>
@@ -802,49 +714,12 @@<br>
     return 0;<br>
 }<br>
<br>
-static int64_t get_pcr(const MpegTSWrite *ts, AVIOContext *pb)<br>
<br>
-static void write_packet(AVFormatContext *s, const uint8_t *packet)<br>
<br>
-static void section_write_packet(MpegTSSection *s, const uint8_t *packet)<br>
<br>
all three gone .... can they stay ? :}<br>
(for reducing diff size)<br>
<br>
 static MpegTSService *mpegts_add_service(AVFormatContext *s, int sid,<br>
-                                         const AVDictionary *metadata,<br>
-                                         AVProgram *program)<br>
+                                         const char *provider_name,<br>
+                                         const char *name)<br>
 {<br>
     MpegTSWrite *ts = s->priv_data;<br>
     MpegTSService *service;<br>
-    AVDictionaryEntry *title, *provider;<br>
-    char default_service_name[32];<br>
-    const char *service_name;<br>
-    const char *provider_name;<br>
-<br>
-    title = av_dict_get(metadata, "service_name", NULL, 0);<br>
-    if (!title)<br>
-        title = av_dict_get(metadata, "title", NULL, 0);<br>
-    snprintf(default_service_name, sizeof(default_service_name), "%s%02d", DEFAULT_SERVICE_NAME, ts->nb_services + 1);<br>
-    service_name  = title ? title->value : default_service_name;<br>
-    provider      = av_dict_get(metadata, "service_provider", NULL, 0);<br>
-    provider_name = provider ? provider->value : DEFAULT_PROVIDER_NAME;<br>
<br>
<br>
I think this one allowed more data passing from ...upper libavcodec layer?<br>
<br>
-static void enable_pcr_generation_for_stream(AVFormatContext *s, AVStream *pcr_st)<br>
+static void mpegts_prefix_m2ts_header(AVFormatContext *s)<br>
 {<br>
     MpegTSWrite *ts = s->priv_data;<br>
-    MpegTSWriteStream *ts_st = pcr_st->priv_data;<br>
-<br>
-    if (ts->mux_rate > 1 || ts->pcr_period_ms >= 0) {<br>
-        int pcr_period_ms = ts->pcr_period_ms == -1 ? PCR_RETRANS_TIME : ts->pcr_period_ms;<br>
-        ts_st->pcr_period = av_rescale(pcr_period_ms, PCR_TIME_BASE, 1000);<br>
-    } else {<br>
-        /* By default, for VBR we select the highest multiple of frame duration which is less than 100 ms. */<br>
-        int64_t frame_period = 0;<br>
-        if (pcr_st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {<br>
-            int frame_size = av_get_audio_frame_duration2(pcr_st->codecpar, 0);<br>
-            if (!frame_size) {<br>
-               av_log(s, AV_LOG_WARNING, "frame size not set\n");<br>
-               frame_size = 512;<br>
-            }<br>
-            frame_period = av_rescale_rnd(frame_size, PCR_TIME_BASE, pcr_st->codecpar->sample_rate, AV_ROUND_UP);<br>
-        } else if (pcr_st->avg_frame_rate.num) {<br>
-            frame_period = av_rescale_rnd(pcr_st->avg_frame_rate.den, PCR_TIME_BASE, pcr_st->avg_frame_rate.num, AV_ROUND_UP);<br>
-        }<br>
-        if (frame_period > 0 && frame_period <= PCR_TIME_BASE / 10)<br>
-            ts_st->pcr_period = frame_period * (PCR_TIME_BASE / 10 / frame_period);<br>
-        else<br>
-            ts_st->pcr_period = 1;<br>
+    if (ts->m2ts_mode) {<br>
+        uint32_t tp_extra_header = ts->pcr % 0x3fffffff;<br>
+        tp_extra_header = AV_RB32(&tp_extra_header);<br>
+        avio_write(s->pb, (unsigned char *) &tp_extra_header,<br>
+                   sizeof(tp_extra_header));<br>
     }<br>
-<br>
-    // output a PCR as soon as possible<br>
-    ts_st->last_pcr   = ts->first_pcr - ts_st->pcr_period;<br>
 }<br>
<br>
May be just leave old function the same, just add short new one<br>
(static void mpegts_prefix_m2ts_header(AVFormatContext *s)) ?<br>
<br>
same for<br>
<br>
-static void select_pcr_streams(AVFormatContext *s)<br>
+static void section_write_packet(MpegTSSection *s, const uint8_t *packet)<br>
<br>
 static int mpegts_init(AVFormatContext *s)<br>
<br>
ah, so av_dict_get and co  moved here ....<br>
<br>
-        /* MPEG pid values < 16 are reserved. Applications which set st->id in<br>
-         * this range are assigned a calculated pid. */<br>
-        if (st->id < 16) {<br>
-            if (ts->m2ts_mode) {<br>
-                switch (st->codecpar->codec_type) {<br>
-                case AVMEDIA_TYPE_VIDEO:<br>
-                    ts_st->pid = ts->m2ts_video_pid++;<br>
+        program = av_find_program_from_stream(s, NULL, i);<br>
+        if (program) {<br>
+            for (j = 0; j < ts->nb_services; j++) {<br>
+                if (ts->services[j]->program == program) {<br>
+                    service = ts->services[j];<br>
                     break;<br>
-                case AVMEDIA_TYPE_AUDIO:<br>
-                    ts_st->pid = ts->m2ts_audio_pid++;<br>
-                    break;<br>
-                case AVMEDIA_TYPE_SUBTITLE:<br>
-                    switch (st->codecpar->codec_id) {<br>
-                    case AV_CODEC_ID_HDMV_PGS_SUBTITLE:<br>
-                        ts_st->pid = ts->m2ts_pgssub_pid++;<br>
-                        break;<br>
-                    case AV_CODEC_ID_HDMV_TEXT_SUBTITLE:<br>
-                        ts_st->pid = ts->m2ts_textsub_pid++;<br>
-                        break;<br>
-                    }<br>
-                    break;<br>
-                }<br>
-                if (ts->m2ts_video_pid   > M2TS_VIDEO_PID + 1          ||<br>
-                    ts->m2ts_audio_pid   > M2TS_AUDIO_START_PID + 32   ||<br>
-                    ts->m2ts_pgssub_pid  > M2TS_PGSSUB_START_PID + 32  ||<br>
-                    ts->m2ts_textsub_pid > M2TS_TEXTSUB_PID + 1        ||<br>
-                    ts_st->pid < 16) {<br>
-                    av_log(s, AV_LOG_ERROR, "Cannot automatically assign PID for stream %d\n", st->index);<br>
-                    return AVERROR(EINVAL);<br>
                 }<br>
-            } else {<br>
-                ts_st->pid = ts->start_pid + i;<br>
             }<br>
-        } else {<br>
-            ts_st->pid = st->id;<br>
         }<br>
<br>
Oh, here our subtitle support go .......<br>
<br>
a lot of skipped, I'm very unsure about all those changes ...<br>
<br>
<br>
@@ -1724,7 +1585,7 @@<br>
<br>
             ret = avio_open_dyn_buf(&ts_st->amux->pb);<br>
             if (ret < 0)<br>
-                return ret;<br>
+                return AVERROR(ENOMEM);<br>
<br>
seems like  possible standalone patch for ffmpeg upstream inclusion?<br>
<br>
@@ -1755,7 +1616,7 @@<br>
         } while (p < buf_end && (state & 0x7e) != 2*35 &&<br>
                  (state & 0x7e) >= 2*32);<br>
<br>
-        if ((state & 0x7e) < 2*16 || (state & 0x7e) >= 2*24)<br>
+        if ((state & 0x7e) < 2*16 && (state & 0x7e) >= 2*24)<br>
<br>
???? why it was changed?<br>
<br>
<br>
<br>
IIRC those changes come from fact ffmpeg's<br>
mpegts muxer mostly geared towards transmission vs standalone BD creation ....<br>
<br>
But at least potential for BD text subtitles look interestinge nough<br>
for keeping looking into this area ...I think.<br>
<br>
Also, ffmpeg 4.3.1 was already released, with small fixes:<br>
<br>
<a href="https://ffmpeg.org/download.html" rel="noreferrer" target="_blank">https://ffmpeg.org/download.html</a><br>
<br>
FFmpeg 4.3.1 "4:3"<br>
 4.3.1 was released on 2020-07-11. It is the latest stable FFmpeg release from the 4.3 release branch, <br>
which was cut from master on 2020-06-08.<br>
<br>
<br>
It was FAST ......<br>
<br>
Also, there is one more patch on ML:<br>
<a href="http://ffmpeg.org/pipermail/ffmpeg-devel/2020-July/266181.html" rel="noreferrer" target="_blank">http://ffmpeg.org/pipermail/ffmpeg-devel/2020-July/266181.html</a><br>
<br>
[FFmpeg-devel] [PATCH] x86/yuv2rgb: fix crashes when storing data on unaligned buffers<br>
<br>
Regression since fc6a5883d6af8cae0e96af84dda0ad74b360a084 on SSSE3 enabled<br>
CPUs.<br>
<br>
Fixes ticket #8747<br>
<br>
-----<br>
<br>
END!<br>
-- <br>
Cin mailing list<br>
<a href="mailto:Cin@lists.cinelerra-gg.org" target="_blank">Cin@lists.cinelerra-gg.org</a><br>
<a href="https://lists.cinelerra-gg.org/mailman/listinfo/cin" rel="noreferrer" target="_blank">https://lists.cinelerra-gg.org/mailman/listinfo/cin</a><br>
</blockquote></div>