This is my work in progress patchset which adds MPEG video parser to MPEG splitter filter. Mainly, fix for bug 9127.
Before moving on the farther step, I'd like to request for comments on this implementation (function name, patch order, GStreamer usage, etc.).
I'll work on MPEG video decoder filter, next. Because, at this point, we can't build a pipeline for MPEG video files with the MPEG splitter. After finishing up the decoder, I'll submit both of patchset. Supporting system stream (audio+video) will be the subsequent development.
Akihiro Sagawa (5): quartz/tests: Factor out MP3 audio tests. winegstreamer: Implement listing of sink media types. winegstreamer: Pass the AM_MEDIA_TYPE parameter to the handler. winegstreamer: Add MPEG video parser to MPEG splitter. quartz/tests: Add MPEG video parser tests for MPEG splitter.
dlls/quartz/tests/mpegsplit.c | 894 +++++++++++++++++++++++++--------- dlls/quartz/tests/rsrc.rc | 4 + dlls/quartz/tests/video.mpg | Bin 0 -> 617 bytes dlls/winegstreamer/gstdemux.c | 192 +++++++- 4 files changed, 838 insertions(+), 252 deletions(-) create mode 100644 dlls/quartz/tests/video.mpg
On 6/21/20 1:40 AM, Akihiro Sagawa wrote:
This is my work in progress patchset which adds MPEG video parser to MPEG splitter filter. Mainly, fix for bug 9127.
Before moving on the farther step, I'd like to request for comments on this implementation (function name, patch order, GStreamer usage, etc.).
I'll work on MPEG video decoder filter, next. Because, at this point, we can't build a pipeline for MPEG video files with the MPEG splitter. After finishing up the decoder, I'll submit both of patchset. Supporting system stream (audio+video) will be the subsequent development.
So, I'm guessing that the application that wants this builds the entire graph themselves? Or do they insert mpegvideoparse and autoplug the rest? If the latter, I kind of wonder if we could wire it up to decodebin instead...
I think patch 1/5 makes sense, though the title is a little awkward. It might be a good idea to split it (it is already a little large), with one patch as something like "quartz/tests: Pass a file name to some MPEG splitter test functions" and one patch as something like "quartz/tests: Separate audio-specific MPEG splitter tests into their own functions."
The rest of the patches look fine at a cursory glance, though see also my question above. I do wonder
I'm not sure it's worth going out of your way to make test_enum_media_types() work with both sinks. The point is just to show that IEnumMediaTypes functions have a certain behaviour across most or all filters.
As always it'd be nice to put tests before fixes (so 5/5 before 4/5, I guess), but if that gets too awkward it'd not a big deal.
Akihiro Sagawa (5): quartz/tests: Factor out MP3 audio tests. winegstreamer: Implement listing of sink media types. winegstreamer: Pass the AM_MEDIA_TYPE parameter to the handler. winegstreamer: Add MPEG video parser to MPEG splitter. quartz/tests: Add MPEG video parser tests for MPEG splitter.
dlls/quartz/tests/mpegsplit.c | 894 +++++++++++++++++++++++++--------- dlls/quartz/tests/rsrc.rc | 4 + dlls/quartz/tests/video.mpg | Bin 0 -> 617 bytes dlls/winegstreamer/gstdemux.c | 192 +++++++- 4 files changed, 838 insertions(+), 252 deletions(-) create mode 100644 dlls/quartz/tests/video.mpg
On Monday, 22 June 2020 23:22:48 +07 you wrote:
On 6/21/20 1:40 AM, Akihiro Sagawa wrote:
This is my work in progress patchset which adds MPEG video parser to MPEG splitter filter. Mainly, fix for bug 9127.
Before moving on the farther step, I'd like to request for comments on this implementation (function name, patch order, GStreamer usage, etc.).
I'll work on MPEG video decoder filter, next. Because, at this point, we can't build a pipeline for MPEG video files with the MPEG splitter. After finishing up the decoder, I'll submit both of patchset. Supporting system stream (audio+video) will be the subsequent development.
So, I'm guessing that the application that wants this builds the entire graph themselves? Or do they insert mpegvideoparse and autoplug the rest? If the latter, I kind of wonder if we could wire it up to decodebin instead...
I don't know which application is Akihiro trying to fix, but I know at least one application that does this: Earth 2150 creates both MPEG splitter and MPEG audio decoder and connects everything explicitly.
I think patch 1/5 makes sense, though the title is a little awkward. It might be a good idea to split it (it is already a little large), with one patch as something like "quartz/tests: Pass a file name to some MPEG splitter test functions" and one patch as something like "quartz/tests: Separate audio-specific MPEG splitter tests into their own functions."
The rest of the patches look fine at a cursory glance, though see also my question above. I do wonder
I'm not sure it's worth going out of your way to make test_enum_media_types() work with both sinks. The point is just to show that IEnumMediaTypes functions have a certain behaviour across most or all filters.
As always it'd be nice to put tests before fixes (so 5/5 before 4/5, I guess), but if that gets too awkward it'd not a big deal.
Akihiro Sagawa (5): quartz/tests: Factor out MP3 audio tests. winegstreamer: Implement listing of sink media types. winegstreamer: Pass the AM_MEDIA_TYPE parameter to the handler. winegstreamer: Add MPEG video parser to MPEG splitter. quartz/tests: Add MPEG video parser tests for MPEG splitter.
dlls/quartz/tests/mpegsplit.c | 894 +++++++++++++++++++++++++--------- dlls/quartz/tests/rsrc.rc | 4 + dlls/quartz/tests/video.mpg | Bin 0 -> 617 bytes dlls/winegstreamer/gstdemux.c | 192 +++++++- 4 files changed, 838 insertions(+), 252 deletions(-) create mode 100644 dlls/quartz/tests/video.mpg
On 6/24/20 11:31 AM, Anton Baskanov wrote:
On Monday, 22 June 2020 23:22:48 +07 you wrote:
On 6/21/20 1:40 AM, Akihiro Sagawa wrote:
This is my work in progress patchset which adds MPEG video parser to MPEG splitter filter. Mainly, fix for bug 9127.
Before moving on the farther step, I'd like to request for comments on this implementation (function name, patch order, GStreamer usage, etc.).
I'll work on MPEG video decoder filter, next. Because, at this point, we can't build a pipeline for MPEG video files with the MPEG splitter. After finishing up the decoder, I'll submit both of patchset. Supporting system stream (audio+video) will be the subsequent development.
So, I'm guessing that the application that wants this builds the entire graph themselves? Or do they insert mpegvideoparse and autoplug the rest? If the latter, I kind of wonder if we could wire it up to decodebin instead...
I don't know which application is Akihiro trying to fix, but I know at least one application that does this: Earth 2150 creates both MPEG splitter and MPEG audio decoder and connects everything explicitly.
Okay, I was afraid that might happen. I guess we'll have to keep all of this code, then :-/
I think patch 1/5 makes sense, though the title is a little awkward. It might be a good idea to split it (it is already a little large), with one patch as something like "quartz/tests: Pass a file name to some MPEG splitter test functions" and one patch as something like "quartz/tests: Separate audio-specific MPEG splitter tests into their own functions."
The rest of the patches look fine at a cursory glance, though see also my question above. I do wonder
I'm not sure it's worth going out of your way to make test_enum_media_types() work with both sinks. The point is just to show that IEnumMediaTypes functions have a certain behaviour across most or all filters.
As always it'd be nice to put tests before fixes (so 5/5 before 4/5, I guess), but if that gets too awkward it'd not a big deal.
Akihiro Sagawa (5): quartz/tests: Factor out MP3 audio tests. winegstreamer: Implement listing of sink media types. winegstreamer: Pass the AM_MEDIA_TYPE parameter to the handler. winegstreamer: Add MPEG video parser to MPEG splitter. quartz/tests: Add MPEG video parser tests for MPEG splitter.
dlls/quartz/tests/mpegsplit.c | 894 +++++++++++++++++++++++++--------- dlls/quartz/tests/rsrc.rc | 4 + dlls/quartz/tests/video.mpg | Bin 0 -> 617 bytes dlls/winegstreamer/gstdemux.c | 192 +++++++- 4 files changed, 838 insertions(+), 252 deletions(-) create mode 100644 dlls/quartz/tests/video.mpg
On Mon, 22 Jun 2020 11:22:48 -0500, Zebediah Figura wrote:
On 6/21/20 1:40 AM, Akihiro Sagawa wrote:
I'll work on MPEG video decoder filter, next. Because, at this point, we can't build a pipeline for MPEG video files with the MPEG splitter. After finishing up the decoder, I'll submit both of patchset. Supporting system stream (audio+video) will be the subsequent development.
So, I'm guessing that the application that wants this builds the entire graph themselves? Or do they insert mpegvideoparse and autoplug the rest? If the latter, I kind of wonder if we could wire it up to decodebin instead...
I greatly appreciate your review and apologize for late response. My main target application is KiriKiri2[1] based visual novel. It creates MPEG splitter/vide/audio filters and connects respectively. KiriKiri2 is released under GPL2, so we can read its source code on GitHub for details[2].
[1] https://en.wikipedia.org/wiki/List_of_visual_novel_engines#KiriKiri [2] https://github.com/krkrz/krkr2/blob/dec49af97e174d31059c3ccd7efc700ba3c6b788...
I'll improve my patchset and submit it with the video filter's patchset when ready.
I think patch 1/5 makes sense, though the title is a little awkward. It might be a good idea to split it (it is already a little large), with one patch as something like "quartz/tests: Pass a file name to some MPEG splitter test functions" and one patch as something like "quartz/tests: Separate audio-specific MPEG splitter tests into their own functions."
The rest of the patches look fine at a cursory glance, though see also my question above. I do wonder
I'm not sure it's worth going out of your way to make test_enum_media_types() work with both sinks. The point is just to show that IEnumMediaTypes functions have a certain behaviour across most or all filters.
As always it'd be nice to put tests before fixes (so 5/5 before 4/5, I guess), but if that gets too awkward it'd not a big deal.
-
Akihiro Sagawa -
Anton Baskanov -
Zebediah Figura