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

Recording always forces yuv420p for h264 codecs even if encoder doesn't support it #2084

Closed
luctowers opened this issue Mar 26, 2021 · 8 comments

Comments

@luctowers
Copy link

Hi, I built mgba-qt from source on Ubuntu Focal. I tried to record with the 'lossless' preset and got an error.

[libx264rgb @ 0x55d9e8876300] Specified pixel format yuv420p is invalid or not supported

found this bug

if (encoder->video->codec->id == AV_CODEC_ID_H264 &&
    (strcasecmp(encoder->containerFormat, "mp4") ||
        strcasecmp(encoder->containerFormat, "m4v") ||
        strcasecmp(encoder->containerFormat, "mov"))) {
    // QuickTime and a few other things require YUV420
    encoder->video->pix_fmt = AV_PIX_FMT_YUV420P;
}

strcasecmp returns zero when strings are equal, this code to assumes the opposite.

@endrift
Copy link
Member

endrift commented Mar 26, 2021

this code does not assume the opposite. wait you're right, it does assume the opposite

The pixel format is supposed to be overridden later in the code if crf is set to zero. Did you set it to lossless and then change the crf? turns out I'm misremembering this code. I'll have a fix soon.

@endrift
Copy link
Member

endrift commented Mar 27, 2021

Thinking about this more, I should probably disallow putting lossless in mp4/m4v/mov, since yuv420p is not lossless.

@luctowers
Copy link
Author

Yeah, it's a bit of rough spot with player compatibility. Having MKVs be the container used for lossless is a little arbitrary too, since programs like windows media player can still play MKVs if they use yuv420p just like it can MP4s. But, I get the motivation of not wanting users to get confused when they can open some MP4s but not others.

@endrift
Copy link
Member

endrift commented Mar 27, 2021

That is indeed the motivation. It's a usability issue, as pointed out in #1693

@luctowers
Copy link
Author

I have an idea that might be interesting, but would take a fair bit of reworking.

Keep the simple of view of record A/V the same or similar, maybe add a couple more knobs that are guaranteed not to bork anything. Then, change the advanced view to be more like OBS's custom FFMPEG output view, where it provides a few main knobs that apply to all encoders, then it provides separate fields for custom parameters that can be passed to the encoders. That way you can give stuff in the simple menu that will behave as expected by layman. Then kind of leave the advanced menu for power-users that know a little about FFMPEG/LIBAV.

Right now the advanced window kind of sits in the uncomfortable spot where it's not flexible enough for power-users. And on the other end gives layman too many opportunities to shoot themselves in the foot and think it's mGBA's fault.

obs screenshot

@endrift
Copy link
Member

endrift commented Mar 27, 2021

kinda the point was to keep even the advanced menu simple enough where I didn't put a wholesale video recording thing in and people who were super power users would just transcode outside of the program. You do have a point though.

@luctowers
Copy link
Author

That's fair. My hopes are probably a little out to lunch. I use mGBA to run streamed casual speedruns tournaments between friends. I basically give them a string like tcp://xxx.xxx.xxx.xxx:xxxx to plug into output field in record A/V and it streams to an ingest on my machine. It actually works pretty well on unmodified mGBA, but I do sometimes need to modify mGBA to add parameters to it. It saves me needing to get non-techy friends to setup OBS on their system lol.

@endrift
Copy link
Member

endrift commented Mar 27, 2021

I think figuring out a better approach for this for 0.10 is a good idea to be honest. But as I have just one point of view having some people to brainstorm with is probably useful.

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

Successfully merging a pull request may close this issue.

2 participants