On Wed, 29 Jul 2026 22:58:31 GMT, Alexander Matveev <[email protected]> wrote:
>> * Replaced `qtdemux` with a Media Foundation-based MP4 demuxer on Windows. >> * The Media Foundation-based MP4 demuxer supports **pull mode only** due to >> the design of the Media Foundation APIs. For fragmented MP4 (fMP4) segments, >> each segment is demuxed independently because of API limitations. After >> processing each segment, the demuxer internally creates a new Source Reader. >> * **HLSConnectionHolder:** Headers are now sent with every segment because >> the MP4 demuxer reloads the Source Reader for each segment. Headers are >> intentionally not cached to avoid unnecessary connections. Additionally, HLS >> streams with separate audio renditions now seek to the last segment when >> seeking to the end of the stream, matching the behavior of standard HLS >> streams. >> * **JavaSource:** Removed the unused `update` flag and moved caps setting >> into a dedicated switch case. >> * Added trace support for Media Foundation-based plugins (both the demuxer >> and decoder). The tracing infrastructure is still a work in progress and >> will be expanded as needed for debugging. It is disabled by default and can >> only be enabled at compile time. >> * H.264 video from MP4 and fMP4 (HLS) is now decoded using Media Foundation >> instead of DirectShow. H.264 video in TS-based HLS streams continues to use >> DirectShow. This change avoids several DirectShow issues encountered with >> the new MP4 demuxer and reduces implementation complexity. >> * Updated `MFWrapper` to properly support H.264 decoding. Unlike H.265, >> H.264 requires using the display aperture for the display resolution, does >> not signal frame availability, requires handling format changes, and must >> fully drain output before accepting new input samples. >> * Modified the cache implementation to support pull mode for the HLS >> progress buffer. >> * Added pull mode support to the HLS progress buffer. >> * Changed `progressbuffer` to use a static source pad instead of a dynamic >> one, simplifying pipeline construction. The need for a dynamic source pad is >> unclear. >> * Performed minor cleanup by removing dead and obsolete code. >> * The progress buffer is now forced when `JavaSource` does not support pull >> mode (for example, with `jar` and `jrt` protocols). This provides a simple >> workaround until native pull mode support is implemented for these >> protocols. In particular, `jar` does not provide true random access at the >> Java layer anyway. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://op... > > Alexander Matveev has updated the pull request incrementally with one > additional commit since the last revision: > > 8305842: Replace qtdemux with MP4 demux based on Media Foundation [v8] I haven't done much testing, but left a few inline comments / questions. modules/javafx.media/src/main/native/gstreamer/gstreamer-lite/projects/plugins/gstplugins-lite.c line 45: > 43: !plugin_init_app(plugin) || > 44: !plugin_init_audioparsers(plugin) || > 45: !plugin_init_qtdemux(plugin)) This used to be called on all platforms. I see you moved this initialization from a platform-independent call to a Linux-only call. That means it won't be called for macOS. Since you also removed qtdemux from the mac Makefile, I presume that this is intentional. Was qtdemux ever used on macOS prior to this? modules/javafx.media/src/main/native/gstreamer/plugins/dshowwrapper/dshowwrapper.cpp line 838: > 836: } > 837: #endif > 838: This file has two whitespace-only changes (removal of two blank lines). I recommend reverting it. modules/javafx.media/src/main/native/gstreamer/plugins/mfwrapper/mfdemux.cpp line 747: > 745: } > 746: > 747: hr = MFCreateSourceReaderFromByteStream(demux->pIMFByteStream, NULL, > &demux->pSourceReader); This is called by `mfdemux_loop` on a task thread. I think you need to initialize COM on this thread (with `CoInitializeEx`). modules/javafx.media/src/main/native/gstreamer/plugins/mfwrapper/mfdemux.cpp line 1452: > 1450: #if TRACE_ENABLE > 1451: TRACE(DEMUX_SRC_EVENTS, "GST_EVENT_SEGMENT start=%lld time=%lld > position=%lld\n", > 1452: segment.start, segment.time, segment.position); `segment` is undefined here. modules/javafx.media/src/main/native/gstreamer/plugins/mfwrapper/mfgstbytestream.cpp line 189: > 187: HRESULT CMFGSTByteStream::Close() > 188: { > 189: // Nothing to close Could there be a problem with a pending async read? If so, do you need to cancel it here? modules/javafx.media/src/main/native/gstreamer/plugins/mfwrapper/mfgstbytestream.cpp line 209: > 207: SafeRelease(&m_pAsyncResult); > 208: > 209: return S_OK; Shouldn't this return `m_readResult`? modules/javafx.media/src/main/native/gstreamer/plugins/mfwrapper/mfwrapper.cpp line 588: > 586: > 587: HRESULT hr = mfwrapper_create_sample_from_gst_buffer(&pSample, buf, > 588: decoder->force_discontinuity); This field is never set. modules/javafx.media/src/main/native/gstreamer/plugins/mfwrapper/mfwrapper.h line 77: > 75: gboolean is_eos; > 76: gboolean is_decoder_initialized; > 77: gboolean force_discontinuity; This field is used, but never set. Is there a reason to add what looks like a duplicate of the existing `is_force_discontinuity` field? modules/javafx.media/src/main/native/gstreamer/plugins/progressbuffer/posix/filecache.c line 177: > 175: > 176: if ((guint64)size < bytes_available) > 177: size = bytes_available; This test looks backwards (compare with `win32/filecache.c` which looks correct) ------------- PR Review: https://git.openjdk.org/jfx/pull/2206#pullrequestreview-4973251414 PR Review Comment: https://git.openjdk.org/jfx/pull/2206#discussion_r3813882240 PR Review Comment: https://git.openjdk.org/jfx/pull/2206#discussion_r3813916856 PR Review Comment: https://git.openjdk.org/jfx/pull/2206#discussion_r3815861049 PR Review Comment: https://git.openjdk.org/jfx/pull/2206#discussion_r3816063307 PR Review Comment: https://git.openjdk.org/jfx/pull/2206#discussion_r3816041381 PR Review Comment: https://git.openjdk.org/jfx/pull/2206#discussion_r3815963337 PR Review Comment: https://git.openjdk.org/jfx/pull/2206#discussion_r3815810096 PR Review Comment: https://git.openjdk.org/jfx/pull/2206#discussion_r3815798294 PR Review Comment: https://git.openjdk.org/jfx/pull/2206#discussion_r3816002741
