lisamelton / video_transcoding

Tools to transcode, inspect and convert videos.
MIT License
2.39k stars 160 forks source link

Add `--mixdown` option to `transcode-video` #245

Closed samhutchins closed 5 years ago

samhutchins commented 5 years ago

I mentioned this idea in my latest comment on #113, and I thought I'd have a crack at implementing it.

I've restricted the options to dpl2 and stereo, because transcode-video only specifies a mixdown to HandBrakeCLI when creating a stereo track from a surround source, and these are the only 2 mixdowns that make sense.

I've left the default as dpl2

lisamelton commented 5 years ago

@samhutchins As usual, you're way ahead of me! :) Thanks for the patch! I'll take a look at it later today.

lisamelton commented 5 years ago

@samhutchins OK, this looks pretty damn good. I'll probably just merge it as is this weekend and make my usual little anal-retentive changes afterwards.

Thanks!

lisamelton commented 5 years ago

@samhutchins Merged! And thanks again. I'll start fiddling with it and probably do a release sometime this week.

samhutchins commented 5 years ago

Cool cool

lisamelton commented 5 years ago

@samhutchins BTW, I haven't released this yet because 1) I suck as a human being :) and 2) I'm planning for it to go in the same version as the "AVBR" ratecontrol system. Which should be no later than next week.

That said, your implementation works flawlessly so I didn't change a line! In fact, the code is exactly how I would have implemented it if I had been clever enough to think of it myself. :)

I did, however, alter the --help text. Mostly so it wouldn't wrap.

One second thought was whether we should make this work per track. But I quickly discounted that as an unnecessary complication. Also, if someone wants to change the mixdown for AAC tracks, they would likely want to change them all anyway.

Another thought is that there might be people with Android-based devices who would want more than two channels in their AAC tracks since, for example, Chromecast supports 5.1 AAC. But adding support for 5point1, 6point1, 7point1, etc. as --mixdown arguments would be, while precise, a pain to apply globally since the user would have to know ahead of time and not make sense for every AAC track.

So, maybe in the future we can add a surround or smart or full argument to your --mixdown option that would just "do the right thing" for AAC. Then again, maybe we should be using a different mechanism for that.

What do you think?

samhutchins commented 5 years ago

@donmelton Release when you're ready man, I'll just used a patched local build until then.

I'm not sure about other audio options, it gets real complicated real fast. I've actually been thinking about other options I want for audio on top of this mixdown option. For example: I've been ripping a lot of TV recently, from both Blu Rays and DVDs, and I end up with audio in a bunch of different formats (raw PCM or AC3 seem to be the most common) and track layouts (2.0 or 5.1). With TV shows, I don't care about maintaining the surround track, so I almost want a --prefer-stereo option that will copy stereo AC3 tracks as is, convert stereo PCM/flac to AAC, and mix 5.1 AC3 down to stereo AAC. In essense, I care more about the output track layout than the format. --audio-width main=stereo seems to transcode stereo AC3 to AAC. But I'm not entirely sure what to name that option, how to describe it, how it interacts with options like --prefer-ac3, and I also worry that the API for transcode-video is becoming quite bloated.

As for changes to --mixdown:
I knew you'd change the help text. ;-). I actually drafted a few versions, and moved it around a bunch of times before opening the pull request. In the end I wanted to get the implementation right, and put the option in the right place in the code/help text. One of these days I'll be able to tune my inner "Don Melton" emulator sufficiently to get the help text right...

I, too, thought about the per-track mixdown and concluded the same as you. Thinking about it more though, I can think of a couple of reasons to do it:

  1. Full surround sound AAC, as you suggested
  2. A secondary language track that you want to be surround, but don't want to spend the bits. i.e., 5.1 AC3 English, 2.0 stereo English, 2.0 dpl2 French

Not sure how to implement such options though. I'd have to noodle on it, as you might say ;-)

lisamelton commented 5 years ago

@samhutchins LOL! Yeah, you need a Turing-complete emulator. :)

Completely agree about needing a --prefer-ac3 equivalent! I've considering adding something like that for awhile because I'm getting so tired of typing --audio-width main=stereo all the time. But I would probably call it --prefer-aac for reasons of symmetry and because we might add surround support in AAC later.

API bloat is a real concern, BTW. Not only because it can confuse users but because it can box you in later on.

And I didn't think of that second point regarding per-track mixdown. Yeah, a secondary language track is a good justification. Or maybe a commentary.

I think we should go with your current implementation first, though. Even if we decide to change or even slightly break the API later.

In the meantime, definitely noodle on it!