lisamelton / video_transcoding

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

Yadif bug #155

Closed maxwilkie closed 6 years ago

maxwilkie commented 7 years ago

Hi Don,

I've been slowly transcoding a bunch of old British television from DVDs over the last few months, which is interlaced material (25fps/50 fields ps at 720x576 resolution) and have been doing so by using:

transcode-video --filter deinterlace=bob '/path/to/movie.mkv'

Some months ago, in April, after upgrading video_transcoding and its dependencies I discovered that the yadif filter was no longer working in HandBrakeCLI versions 1.0.0 (this seems to be a known bug) and above, so I simply reverted to version 0.10.5 and kept all other dependencies, as well as video_transcoding, updated.

Tonight (5th July) I again updated video_transcoding (to version 0.17.3) and all dependencies other than HandBrake and again found that the interlaced material was not deinterlacing correctly. Thinking this might have something to do with the new approach to frame rate modes, I tried upgrading HandBrake to version 1.0.7, though the issue persisted. I've reverted back to video_transcoding 0.17.2 and HandBrakeCLI 0.10.5 and the videos are deinterlacing correctly again.

Cheers, Max

lisamelton commented 7 years ago

@maxwilkie Thank you for the detailed bug report and my apologies for you having to deal with this problem.

Although I tested the small change in version 0.17.3 with older versions of HandBrake, I didn't specifically check your particular usage of the deinterlace filter. That'll teach me to be more thorough. And I was not aware of issues with the yadif implementation in current versions of HandBrake. Thanks for the heads up on that.

OK, let's see if we can work around this problem for you now until I can figure out a way to fix it. Can you try adding --handbrake-option cfr to your command line like this?

transcode-video --filter deinterlace=bob --handbrake-option cfr '/path/to/movie.mkv'

... and let me know if that fixes it? Thanks.

maxwilkie commented 7 years ago

@donmelton Unfortunately that doesn't fix it. Thanks for your reply!

lisamelton commented 7 years ago

@maxwilkie Thanks for testing that so quickly! But now I'm scratching my head because that should have fixed it. OK, ignore the --handbrake-option cfr trick for now. Instead, can you do me a favor and add the --dry-run option to your command line? Something like this:

transcode-video --dry-run --filter deinterlace=bob '/path/to/movie.mkv'

That will print out what transcode-video plans to send to HandBrakeCLI. Then just paste that output in here. Thanks.

maxwilkie commented 7 years ago

@donmelton Hi Don, I've done a dry run under 0.17.2 and 0.17.3 for comparison:

Here's the 0.17.3 (not performing a bob deinterlace correctly):

transcode-video --dry-run --filter deinterlace=bob /Users/Max/Movies/Not\ Transcoded/Part\ 2.mkv
HandBrakeCLI --input=/Users/Max/Movies/Not\ Transcoded/Part\ 2.mkv --output=Part\ 2.mkv --markers --encoder=x264 --crop=0:0:0:0 --strict-anamorphic --rate=25.0 --cfr --encoder-preset=medium --encoder-profile=high --encoder-level=3.0 --quality=1 --audio=1 --aencoder=ca_aac --encopts=vbv-maxrate=1500:vbv-bufsize=3000:crf-max=25:qpmax=34 --deinterlace=bob

And here is is working correctly in 0.17.2:

transcode-video --dry-run --filter deinterlace=bob /Users/Max/Movies/Not\ Transcoded/Part\ 2.mkv 
HandBrakeCLI --input=/Users/Max/Movies/Not\ Transcoded/Part\ 2.mkv --output=Part\ 2.mkv --markers --encoder=x264 --crop=0:0:0:0 --strict-anamorphic --rate=25.0 --encoder-preset=medium --encoder-profile=high --encoder-level=3.0 --quality=1 --audio=1 --aencoder=ca_aac --encopts=vbv-maxrate=1500:vbv-bufsize=3000:crf-max=25:qpmax=34 --deinterlace=bob

Cheers, Max

lisamelton commented 7 years ago

@maxwilkie Sweet and thanks! And I see the problem now. OK, try this with version 0.17.3:

transcode-video --filter deinterlace=bob --handbrake-option _cfr '/path/to/movie.mkv'

I got it backwards before. Apparently we need to disable forcing the constant frame rate. That underscore is not a typo. The option really is --handbrake-option _cfr to do that.

Let me know if that works. Thanks.

maxwilkie commented 7 years ago

@donmelton Thanks - that works!

After successfully completing that transcode, I ran it under the same settings, but running HandBrakeCLI 1.0.7 on my secondary machine (as expected, yadif didn't bob deinterlace this time) more for the log file than anything else - have attached logs (as txt files) for the successful transcode using HandBrake 0.10.5 and the unsuccessful one using 1.0.7, should you be interested.

Cheers, Max

Yadif_hb0.10.5.txt Yadif_hb1.0.7.txt

lisamelton commented 7 years ago

@maxwilkie Whew! I'm glad that worked. :) And thanks for testing and including those .log files. I'll take a look at those later today.

Now the big question is whether I can make a code change so you don't have to add that option for your case while at the same time not breaking other behaviors when using HandBrake version 1.0 and later for everyone else. Especially not breaking the regression I was actually trying to fix in 1.0.7.

Other than special casing the code based on the current HandBrake version, there's no easy way to do this. And correctly and consistently determining the current HandBrake version is close to impossible. So, this may not be fixable. But since this is only an issue for those using legacy versions of HandBrake (like yourself), then maybe just documenting the workaround is good enough. Especially since you're the only person I know using a legacy version.

I'll have to think about this for awhile.

samhutchins commented 7 years ago

@donmelton If I may ask an unrelated question (I don't know a better place to ask):

Why is determining the version of HandBrake so difficult? Does the syntax of HandBrakeCLI --version change a lot? Are you trying to support nightlies?

lisamelton commented 7 years ago

@samhutchins Good question! But the answer is really complicated. :)

First, there's no --version option support prior HandBrakeCLI 1.0. As you can see in my handbrake.rb source file, you have to parse the output of the --preset-list option for those.

But, even that first rule is not ironclad because of 1) weirdness of nightly builds (which many users run) and 2) the HandBrake development team has the annoying habit of breaking the versioning mechanism.

Then, if you can actually determine the version string, there's the problem of parsing it into an actual number that you can compare against. And if it's a nightly or, worse, development build, then you might as well shoot yourself. :) Seriously, it's that complicated.

maxwilkie commented 7 years ago

@donmelton Thanks for all your help - I'll just keep forcing cfr off for now. Hopefully there's a patch on the way for HandBrake - I'd rather not keep using the legacy version, but so long as I have all this interlaced content to transcode I'll have to.

Thanks again.

Cheers, Max

lisamelton commented 7 years ago

@maxwilkie You are very welcome, sir! I'll leave this issue open for awhile to track the problem although I may reclassify it.

lisamelton commented 6 years ago

@maxwilkie Now that we're all using HandBrake version 1.0 (and probably 1.1) or later, is this still an issue. If not, I'd like close this.

maxwilkie commented 6 years ago

Hi Don,

I’m away from home until the 31st October - will take a look once I’m back.

Thanks for following up.

Cheers, Max

Sent from my iPhone

On 21 Oct 2018, at 4:05 am, Don Melton notifications@github.com wrote:

@maxwilkie Now that we're all using HandBrake version 1.0 (and probably 1.1) or later, is this still an issue. If not, I'd like close this.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

maxwilkie commented 6 years ago

Hi Don,

The short of it seems to be that this is still an issue.

I've run the same video file through three transcodes - two using video_transcoding 0.20.1 & hb 1.1 on my partner's computer (once using the _cfr option, once not) - neither of these worked - the 50i interlaced material appeared to have been deinterlaced to 25p. Ran again on our primary machine, which still uses video_transcoding 0.17.2 & hb 0.10.5, and as expected it looked like 50i material.

I've attached all three logs - hope these are of some use. log hb1.1.log log _cfr hb1.1.log log hb0.10.5.log

Hope you are well, and thanks again.

Cheers, Max

lisamelton commented 6 years ago

@maxwilkie Sorry this is still a problem! And thanks for including those .log files.

As near as I can tell based on the output bitrates and without examining the output files with a tool like MediaInfo, they're all getting transcoded to 25 FPS progressive. So while it may look like interlaced 50 FPS interlaced, it's not. Without special x264 trickery, HandBrake can't generate interlaced output.

The file transcoded by HandBrake version 0.10.5 probably looks better due to their previous deinterlace filter implementation. Again, because this is an internal HandBrake issue, there's not much I can do about it.

Other than using this command which I mentioned before:

transcode-video --filter deinterlace=bob --handbrake-option _cfr '/path/to/movie.mkv'

...I don't see a good solution. Sorry about that.

maxwilkie commented 6 years ago

Hi Don,

Thanks again. Should have been a bit clearer in my previous response - I’m aware that regardless of what the output looks like, it’s all being deinterlaced to 25p; but yes, the yadif implementation in 0.10.5 does so in a way that maintains the “look”, if you will, of 50i material (not being an expert on deinterlacing algorithms I can’t speak to how it does this).

I did try using --handbrake-option _cfr in one of the transcodes using handbrake 1.1 but this still didn’t produce the desired result.

Regardless, yhanks for your help with this issue. I’ll continue to run the legacy versions of video_transcoding and handbrake on one machine for these files, and the current versions on another for anything else I might wish to transcode; and I’ll try to investigate why this is a problem with handbrake directly as well.

Again, many thanks, and all the best.

-Max

lisamelton commented 6 years ago

@maxwilkie Thanks! Sorry I couldn't solve this for you.

I'll close this now but feel free to comment here again or open a new issue.