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

Automatic Cropping #2219

Open
brownbat opened this issue Aug 3, 2019 · 17 comments

Comments

@brownbat
Copy link

commented Aug 3, 2019

Describe the change or feature you'd like to see added to HandBrake:

Generally - improve automatic cropping to make it less likely to crop content.

Current: Black bar detection currently takes the median value after sampling several frames. Recommended: set automatic values to the minimum width of black bars found in the sample frames.

Rationale: black bars only rarely fluctuate in width, but a significant amount of content uses very dark palettes, or a mix of aspect ratios, in which case the current algorithm is confused and becomes overaggressive.

There is a balance between preserving content and preventing artifacts / encoding black pixels. In my experience, and judging from the forums, we could nudge the balance slightly more towards retaining content and be in a better default place for most users.

What version of HandBrake are you currently using? (e.g., 1.0.0)

1.2.2

What operating system and version are you running? (e.g., Ubuntu 16.04 LTS, macOS 10.3 High Sierra, Windows 10 Creators Update)

Windows 10 Pro (build 17134.885)

HandBrake Activity Log (see https://handbrake.fr/docs/en/latest/help/activity-log.html)

N/A

@sr55

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2019

This comes up so exceedingly rarely that it doesn't seem like it's worth investing time into.
Feel free to play around with tuning the algorithm if you want, but you'll likely just end up shifting the under/overcrop balance rather than actually improving it. Typically throwing more samples at it (which is already an option) improves the reliability in the edge cases.

In the more extreme cases, there is always manual crop and I'd argue working on #959 would be more beneficial and quicker than trying different crop algorithms. Not to mention making spot checking easier.

@bradleysepos

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2019

For mixed content, median can and will ignore outliers causing aggressive overcropping. We could probably consider analysis of a single frame or more that's severely different and use that for cropping, but would require significant testing.

@brownbat

This comment has been minimized.

Copy link
Author

commented Aug 3, 2019

#959

This looks like a great contribution, genuinely hope it gets incorporated across versions. If we can make the automatic crop smarter at the same time as you are making it easier to visually confirm, so much the better. :)

exceedingly rarely

Ha, fair, I'm probably a magnet for edge cases, but I'm only here because it keeps dominating my time with the program. I hit it on dark content and content with mixed aspect ratios. Oddly enough, that's most of the DC catalog. Wes Anderson and horror movies are other contenders. But even without weird directorial choices, many DVDs and BRs have some mixed AR special feature. There's often a clip or two that cuts between shots of the film and shots of an interview panel or from behind the scenes production.

The biggest pain was encoding Batman the Animated Series, which admittedly not everyone will do, but illustrates just how much of a pain this can become. That show is so dark, automatic kept wanting to cut out half of the screen. Cranking the sample rate up helped a bit, but didn't solve the issue. Going manual, zeroes weren't the right answer either, and the show wasn't perfectly consistent from episode to episode. So there I was, manually aligning cropping borders pixel by pixel for around 60 episodes. If I can spare one user that tedious nightmare, I'd feel like it was a win. (But I'm done with that show, and I'm still hitting it on about one out of every three discs from films when I include special features.)

Feel free to play around with tuning the algorithm if you want, but you'll likely just end up shifting the under/overcrop balance rather than actually improving it.

From a search through the code, it looks like there's already a backup algorithm called "loose cropping."

It seems like this dead "loose crop" code would be better in all the cases I've encountered so far. I understand there could be a risk of undercropping, but that's going to come up only when there's some floating bright pixel on the edge of the source? I can imagine that happening, but haven't ever seen it.

Another way to look at it -- if we're balancing, I'd expect to have problems with undercropping almost as often as overcropping, but right now I'm only seeing overcropping issues. I'm not even sure 50/50 would be the perfect answer -- a bias against losing actual content might be better, since you can always run another pass and crop more, but you can't uncrop lost information.

But ok, assuming undercrop is an issue worth avoiding, maybe just adding "loose crop" as an option in advanced settings, leaving the current behavior as default, would work without breaking anything for anybody? Bit of a compromise. What do you think?

@bradleysepos

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2019

Loose crop, if I recall correctly, has to do with ensuring modulus without scaling the picture, and it's only on Linux/GTK GUI right now.

@brownbat

This comment has been minimized.

Copy link
Author

commented Aug 3, 2019

Loose crop, if I recall correctly, has to do with ensuring modulus without scaling the picture, and it's only on Linux/GTK GUI right now.

Is that maybe loose anamorphic you're thinking of? Maybe I'm misreading the code, here's the section I'm looking at:

sort_crops( crops );

        // don't try to crop unless we got at least 3 previews
        if ( crops->n > 2 )
        {
            sort_crops( crops );
            // The next line selects median cropping - at least
            // 50% of the frames will have their borders removed.
            // Other possible choices are loose cropping (i = 0) where
            // no non-black pixels will be cropped from any frame and a
            // tight cropping (i = crops->n - (crops->n >> 2)) where at
            // least 75% of the frames will have their borders removed.
            i = crops->n >> 1;
            title->crop[0] = EVEN( crops->t[i] );
            title->crop[1] = EVEN( crops->b[i] );
            title->crop[2] = EVEN( crops->l[i] );
            title->crop[3] = EVEN( crops->r[i] );
        }

We could probably consider analysis of a single frame or more that's severely different and use that for cropping, but would require significant testing.

I considered that too. You might get a good reference frame for many films by offsetting the first preview frame. Typically now it's a black frame, but you might land on a nice bright studio logo if the first frame was pulled from a few seconds in.

But... your "significant testing" line is right, based on the enormous variety of content out there, this would be a nightmare to tune. And a reference frame could still cause issues with mixed A/R.

Given how complicated a "smart" cropping algorithm could get, I think the dev instincts are right to keep it simple here. Which leaves median, lazy, and greedy. Say "greedy" just tries to crop if any of the frames look black and overcrops everything, so it's not helpful. Median works great for normal content: where there are a few dark frames, a few light frames, but those are outliers and most of the content is somewhere in between.

Lazy, or "loose" here, I think would work well for that standard reference content, but also adapt very well to dark content and mixed A/R. It would undercrop on sources that have artifacts inside the black bars, but I think that's rarer than the other cases? And undercropping in general seems less dangerous than overcropping to me.

But that's just like, my opinion, man. :) So I could really see this being softly introduced through a nondefault option in settings. Definitely wouldn't want to move too fast here and break anything.

@bradleysepos

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2019

@jstebbins Would know, pretty sure he wrote that.

@jstebbins

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2019

Loose cropping does what @bradleysepos says. I created it to prevent scaling the image by some small amount due to the modulus requirement. I.e. it crops to the nearest even modulus boundary so that scaling down to the nearest modulus isn't necessary since scaling impacts encode speed and output quality.

@jstebbins

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2019

To fill in some detail. modulus is mostly not needed any more. A modulus of 2 works on pretty much any device you might care about these days. But it used to be that there were more devices that were really dumb and required output resolution to be some specific multiple, usually 16. We could probably eliminate both modulus and loose cropping and never have anyone miss either of them.

@jstebbins

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2019

The code you pasted has nothing to do with loose cropping. That code is simply picking the median sample from the arrays that store the crop samples.

The crop values for each preview frame are recorded in 4 arrays (top, bottom, left, right). Each crop array is sorted by the amount of crop found. And finally the middle sample in each array is chosen as the autocrop value.

@brownbat

This comment has been minimized.

Copy link
Author

commented Aug 3, 2019

The code you pasted has nothing to do with loose cropping.

Other possible choices are loose cropping (i = 0) where
            // no non-black pixels will be cropped from any frame

Ok, I get that I'm using the wrong name for what I'm describing, but in fairness I took it exactly from the code comment.

Sorry for the confusion either way.

What should we call the i=0 option in the code comment, to avoid confusion? Lazy cropping? Conservative cropping?

Also, jstebbins, thanks for all your work here and your explanations on the forums, they've been really helpful.

EDIT: I kind of like "Indy cropping" because it might cause some trouble while preserving artifacts.

@brownbat

This comment has been minimized.

Copy link
Author

commented Aug 3, 2019

Wait, more importantly, why did you choose median rather than i=0? Were you running into a lot of undercropping errors? Really curious what your reference material was, I haven't hit undercropping at all, but hit overcropping quite a bit.

@jstebbins

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2019

I think we were assuming you were talking about the option available in the gui. The comment only points out other options that could be trivially extracted from the array information as it currently exists. But only the median choice is currently available.

The "loose" option described in the comment would cause problems for many lower quality encodes (e.g. old DVD TV shows) that have borders that are not always entirely black and not always in exactly the same place due to how they were transferred to DVD. Old analog processes and such. It would be a poor default choice but could certainly be exposed as an option.

I also like the idea of narrowing the cropped region if some number of frames have narrower than the median crop detected and the difference is "significant" by some definition of significant. I.e. a couple pixels is not significant, but 10% of width or height is. This has the benefit of not affecting the results for video that has poorly defined black bar boundaries, but still fixing the cases like Batman The Dark Knight where the film aspect changes partway through the movie.

@jstebbins

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2019

Were you running into a lot of undercropping errors?

Yes, poorly defined boundaries and boundaries that move by a pixel or 2 on occasion trip up i=0. You end up with a lot of video that has a 2 pixel black bar on one side typically. Old DVDs are the biggest source of such video, and HandBrake was originally developed when these where the most common source of video 😉

@jstebbins

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2019

Another thing to keep in mind. A lot of folks transcode content captured from broadcast. The broadcasts will frequently have transitory logos and such that appear in the black bar. Most folks would rather this is cropped down to the aspect of the movie (and the logo cropped out) rather than narrowing the cropped region and retaining the logo in the output.

@Synapto

This comment has been minimized.

Copy link

commented Aug 4, 2019

This is why I created #2009, which said:

I always resize Preview so that there is a white surround to the image. This makes it easier to visually identify overscan areas (typically 1 or 2 pixels wide) that aren't picked up by autocrop.

I never rely on autocrop. All I use autocrop for is quickly populating the crop fields, then swapping back to "Custom" to verify the values, using the white bordered preview. With 30-60 previews, this becomes a breeze.

The autocrop in HandBrake is fairly good. 95% of the time, it picks the correct crop values, even in gloomy sources.

Things that swap aspect ratio willy-nilly, like the TV series Legion, or any IMAX source, can result in overcropping. I would have thought that increasing the minimum number and spectrum of compared previews would solve this. But, again, I only use autocrop for quickly populating the crop fields which I then manually verify, so even an IMAX Transformers (ugh!) doesn't bother me.

BTW, regarding #959, I tend to dislike overlays on previews, because I'm always left wondering "Can I trust that this overlay isn't concealing something that I actually want to see?" The answer is usually "no".

Which is why I use the white border on the preview window. That way, the only thing visible between the preview margin and the source info is the black crop region, and not some "marching ant" rubbish or confusingly contrasted solid lines that you're always trying to "look under".

@brownbat

This comment has been minimized.

Copy link
Author

commented Aug 4, 2019

Old DVDs make a lot of sense and sound tricky. Broadcast, well, deliberately removing information from the source still feels like a different operation than trimming black space to me, but I definitely get why people would want that.

narrowing the cropped region if some number of frames [are significantly] narrower than the median crop

Interesting. Or putting that another way -- it's almost like you want to rate cropping values on two scales - frequency and smallness. Some weighted combination of those two. Though to your point, first consolidate very similar values.

Some sample crop arrays:
[0, 10, 10, 10, 10, 8, 12, 5]
You currently get 10, which seems right.

Mixed A/R content array:
[0, 10, 10, 10, 10, 8, 12, 5, 5, 5]
In this case, you probably want 5, even though 8 is the median and 10 is the mode.

Old DVD array:
[0, 10, 10, 10, 10, 8, 12, 9, 9, 9]
Here I think you are recommending 10, to avoid adding a 1 px bar to half the video.

That seem like it catches all the major cases?

(Well, except for dark content, which I have no idea how to emulate, so setting that aside for now.)

@giddie

This comment has been minimized.

Copy link

commented Aug 4, 2019

Just weighing in briefly to mention that I also would at least like the option to choose minimum crop area instead of median. I run into this constantly with Blu-Ray special features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
You can’t perform that action at this time.