Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't add subtitles stream to mkv #24

Closed
rkfg opened this issue May 3, 2017 · 3 comments
Closed

Can't add subtitles stream to mkv #24

rkfg opened this issue May 3, 2017 · 3 comments

Comments

@rkfg
Copy link
Contributor

rkfg commented May 3, 2017

I'm using Debian Stretch amd64, libavformat 57.
These lines block adding a subtitles stream. I can form a valid H264+Vorbis stream packed to MKV but can't add subtitles due to that support check. MKV indeed supports subtitles but the check says otherwise. It could be an ffmpeg bug or something with my local setup. I commented the check out, after that avformat_new_stream successfully adds the subtitles stream and I can encode my subs to it getting a valid MKV file.

The code is very straightforward, like this:

        Codec sub_codec = findEncodingCodec(AV_CODEC_ID_SUBRIP);
        auto sub_stream = ofctx.addStream(sub_codec, ec);
        LOGFATAL("Can't add subtitles stream")
        subctx.openInput(argv[4], ec);
        LOGFATAL("Can't open subtitles")
        subctx.findStreamInfo(ec);
        LOGFATAL("No subtitles streams found")

        ofctx.dump();
        ofctx.writeHeader();
        ofctx.flush();

        while (true) {
            auto pkt = subctx.readPacket(ec);
            if (pkt.isNull()) {
                break;
            }
            LOGERR("Packet reading error")
            pkt.setStreamIndex(2);
            ofctx.writePacket(pkt, ec);
            LOGERR("Packet writing error")
        }

I think we shouldn't explicitly check for stream support and instead rely on avformat_new_stream returning NULL on error.

rkfg pushed a commit to rkfg/avcpp that referenced this issue May 12, 2017
rkfg pushed a commit to rkfg/avcpp that referenced this issue Jul 7, 2017
rkfg pushed a commit to rkfg/avcpp that referenced this issue Jul 13, 2017
rkfg pushed a commit to rkfg/avcpp that referenced this issue Nov 24, 2017
@rkfg
Copy link
Contributor Author

rkfg commented Nov 27, 2017

I'm rebasing my branch with some patches so this issue gets spammed. Anyway, it seems something inside libav is not behaving properly. Either MKV is not well-defined in terms of supported formats or av_codec_get_tag2 should not be used like that. Maybe it doesn't support getting tags for subtitle streams, I have no idea really. The fact is, it prevents adding aubtitles to an MKV container and it's wrong so I removed that check. It's redundant anyway because right after adding a stream we check the result for NULL, so if it didn't work for whatever reason we'd throw_if.

@h4tr3d
Copy link
Owner

h4tr3d commented Nov 27, 2017

Do you use libav? What is happen if you just replace it with ffmpeg? ;-)

@h4tr3d h4tr3d closed this as completed in 86bc120 Nov 27, 2017
h4tr3d added a commit that referenced this issue Nov 27, 2017
It seems something inside libav is not behaving properly. Either MKV is not well-defined in terms of supported formats or av_codec_get_tag2 should not be used like that[1]. Maybe it doesn't support getting tags for subtitle streams, I have no idea really. The fact is, it prevents adding aubtitles to an MKV container and it's wrong so I removed that check. It's redundant anyway because right after adding a stream we check the result for NULL, so if it didn't work for whatever reason we'd throw_if.

[1] https://github.com/h4tr3d/avcpp/blob/a2be4c78fe91535498e956b089ef12cc4b5e60e5/src/format.cpp#L14

Fix #24
@rkfg
Copy link
Contributor Author

rkfg commented Nov 27, 2017

Uhh, no, I'm using the mainstream FFmpeg. The problem is googling the information about the C API that's also called ffmpeg as the CLI utility (which is far more popular). So I usually use libav in queries and the result is mostly what I need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants