| Message ID | CANgLvuC21How_=KL+GD6ZUbHVZ4+=Tfjiq2q-w7x7teQS7D4Lw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/matroskaenc: add matroska subtitle muxer (.mks) | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
On 4/4/20, Jan Chren (rindeal) <dev.rindeal@gmail.com> wrote: > > Looks like you sent exactly same patch as before. This patch does unfortunately nothing as some changes from other files are missing.
On Sat, 4 Apr 2020 at 15:56, Paul B Mahol <onemda@gmail.com> wrote: > > Looks like you sent exactly same patch as before. > This is actually the third time I sent it, but first time it went through to Patchwork, which is what counts. > This patch does unfortunately nothing as some changes from other files > are missing. Do you happen to know what the "other files" might be?
Jan Chren (rindeal): > On Sat, 4 Apr 2020 at 15:56, Paul B Mahol <onemda@gmail.com> wrote: >> >> Looks like you sent exactly same patch as before. >> > > This is actually the third time I sent it, but first time it went > through to Patchwork, which is what counts. > >> This patch does unfortunately nothing as some changes from other files >> are missing. > > Do you happen to know what the "other files" might be? You should read my answers to your earlier mails. - Andreas
On Sat, 4 Apr 2020 at 16:32, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote: > > You should read my answers to your earlier mails. Any earlier mails have not reached either my mailbox or Patchwork. But hopefully I have found them in the public mail archive now. Terrible workflow got even worse... > > - Andreas
On Sat, 4 Apr 2020 at 16:32, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote: > What is the aim of your patch? If it is simply making sure that using > the mks extension autoselects the Matroska muxer, That's correct. > then adding "mks" to > the list of extensions of the ordinary Matroska muxer would be enough. > (Notice that using mka for files with audio, but without video or mks > for subtitle-only files is just a convention; putting something else > than indicated by the extension in the files does not make them less > spec-compliant.) Should I create a new patch which removes the audio muxer and just adds `mka` and `mks` extensions to the ordinary muxer? Or is there some reason why a dedicated audio muxer must exist, whereas a subtitle muxer does not?
Am Sa., 4. Apr. 2020 um 20:01 Uhr schrieb Jan Chren (rindeal) <dev.rindeal@gmail.com>: > > On Sat, 4 Apr 2020 at 16:32, Andreas Rheinhardt > <andreas.rheinhardt@gmail.com> wrote: > > What is the aim of your patch? If it is simply making sure that using > > the mks extension autoselects the Matroska muxer, > > That's correct. > > > then adding "mks" to > > the list of extensions of the ordinary Matroska muxer would be enough. > > (Notice that using mka for files with audio, but without video or mks > > for subtitle-only files is just a convention; putting something else > > than indicated by the extension in the files does not make them less > > spec-compliant.) > > Should I create a new patch which removes the audio muxer and just > adds `mka` No. Carl Eugen
On 4/4/2020 3:00 PM, Jan Chren (rindeal) wrote: > On Sat, 4 Apr 2020 at 16:32, Andreas Rheinhardt > <andreas.rheinhardt@gmail.com> wrote: >> What is the aim of your patch? If it is simply making sure that using >> the mks extension autoselects the Matroska muxer, > > That's correct. > >> then adding "mks" to >> the list of extensions of the ordinary Matroska muxer would be enough. >> (Notice that using mka for files with audio, but without video or mks >> for subtitle-only files is just a convention; putting something else >> than indicated by the extension in the files does not make them less >> spec-compliant.) > > Should I create a new patch which removes the audio muxer and just > adds `mka` and `mks` extensions to the ordinary muxer? > Or is there some reason why a dedicated audio muxer must exist, > whereas a subtitle muxer does not? Is there a mimetype specific for .mks? If so, then a separate demuxer would be acceptable. Otherwise, simply adding the extension to the existing matroska muxer and adding stream checks to mkv_init() should suffice. In any case, your patch is apparently not correct as it's missing all the allowed codec ids in AVCodec.codec_tag, and/or defining an AVCodec.query_codec function to only accept subtitle codecs.
James Almer: > On 4/4/2020 3:00 PM, Jan Chren (rindeal) wrote: >> On Sat, 4 Apr 2020 at 16:32, Andreas Rheinhardt >> <andreas.rheinhardt@gmail.com> wrote: >>> What is the aim of your patch? If it is simply making sure that using >>> the mks extension autoselects the Matroska muxer, >> >> That's correct. >> >>> then adding "mks" to >>> the list of extensions of the ordinary Matroska muxer would be enough. >>> (Notice that using mka for files with audio, but without video or mks >>> for subtitle-only files is just a convention; putting something else >>> than indicated by the extension in the files does not make them less >>> spec-compliant.) >> >> Should I create a new patch which removes the audio muxer and just >> adds `mka` and `mks` extensions to the ordinary muxer? >> Or is there some reason why a dedicated audio muxer must exist, >> whereas a subtitle muxer does not? > > Is there a mimetype specific for .mks? If so, then a separate demuxer > would be acceptable. Otherwise, simply adding the extension to the > existing matroska muxer and adding stream checks to mkv_init() should > suffice. > > In any case, your patch is apparently not correct as it's missing all > the allowed codec ids in AVCodec.codec_tag, and/or defining an > AVCodec.query_codec function to only accept subtitle codecs. Using mks is actually just a convention, so putting only subtitles into it should not be enforced (just as is with the Matroska audio muxer). If I am not mistaken, then a separate muxer would only help with stream auto-selection (i.e. it would make -map in FFmpeg cli superfluous). - Andreas
From eaa9f2685436725e19eb94e7f2d0174ba56b7429 Mon Sep 17 00:00:00 2001 From: Jan Chren (rindeal) <dev.rindeal@gmail.com> Date: Thu, 1 Apr 2020 00:00:00 +0000 Subject: [PATCH] avformat/matroskaenc: add matroska subtitle muxer (.mks) Signed-off-by: Jan Chren (rindeal) <dev.rindeal@gmail.com> --- libavformat/matroskaenc.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 5dae53026d8..62127fe97f0 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -2793,3 +2793,31 @@ AVOutputFormat ff_matroska_audio_muxer = { .priv_class = &mka_class, }; #endif + +#if CONFIG_MATROSKA_SUBTITLE_MUXER +static const AVClass mks_class = { + .class_name = "matroska subtitle muxer", + .item_name = av_default_item_name, + .option = options, + .version = LIBAVUTIL_VERSION_INT, +}; +AVOutputFormat ff_matroska_subtitle_muxer = { + .name = "matroska", + .long_name = NULL_IF_CONFIG_SMALL("Matroska Subtitles"), + .extensions = "mks", + .priv_data_size = sizeof(MatroskaMuxContext), + .subtitle_codec = AV_CODEC_ID_ASS, + .init = mkv_init, + .deinit = mkv_deinit, + .write_header = mkv_write_header, + .write_packet = mkv_write_flush_packet, + .write_trailer = mkv_write_trailer, + .check_bitstream = mkv_check_bitstream, + .flags = AVFMT_GLOBALHEADER | AVFMT_VARIABLE_FPS | + AVFMT_TS_NONSTRICT | AVFMT_ALLOW_FLUSH, + .codec_tag = (const AVCodecTag* const []){ + additional_subtitle_tags, 0 + }, + .priv_class = &mks_class, +}; +#endif