Feature: Format filter: Rotation-independent measurements longside and shortside to help with portrait video

[mk-pmb]

Checklist

• I'm reporting a feature request
• I've verified that I'm running youtube-dl version 2021.12.17
• I've searched the bugtracker for similar feature requests including closed ones

Description

I used to filter formats by height, but more and more often, this trips up due to portrait video (height > width).

Rather than making complicated rules about combinations of height and width, I'd like to have rotation-independent measurements longside (=max(w,h)) and shortside (=min(w,h)) available for my format filter.

Patch

Available, albeit far from ideal, see discussion below.

[mk-pmb]

# Portrait video: https://youtu.be/Y5iif7YskU4
$ ytdl --list-formats Y5iif7YskU4 | grep -Pe '^18'
18           mp4        360x640    360p  593k , avc1.42001E, 30fps, mp4a.40.2 (48000Hz), 4.18MiB (best)

# Landscape video: https://youtu.be/fCDsn7OTNMg
$ ytdl --list-formats fCDsn7OTNMg | grep -Pe '^18'
18           mp4        640x360    360p  471k , avc1.42001E, 25fps, mp4a.40.2 (44100Hz), 44.46MiB

# Current approach (fails for portrait video):
$ ytdl --format='best[width<=720][height<=360]' -- Y5iif7YskU4 fCDsn7OTNMg |& grep error
E: error type: 'DownloadError' msg: 'ERROR: requested format not available'

# Intended approach (not yet supported):
$ ytdl --format='best[longside<=720][shortside<=360]' -- Y5iif7YskU4 fCDsn7OTNMg |& grep error
E: error type: 'ValueError' msg: "Invalid filter specification 'longside<=720'"

[mk-pmb]

That would be a better work-around for YouTube, thanks for the idea!

Still, rather than use a single-usecase special workaround, I'd prefer to express my actual intent, so it will work for other websites as well.

[mk-pmb]

to solve imagined issues

We seem to have different ideas about what to strive for in user interface design.

At first I wondered why such a simple addition warrants that much resistance. So I tried to make my own patch and share it here, for the benefit of all who value good UID.

Then I found the parts of code where to add it. I see now. Maybe some time in the future someone will have enough time to refactor it so it will be easy to maintain, so the patch I intended to offer would have a better chance of being applicable to future versions as well.

In current master (6508688) it seems to be youtube_dl/YoutubeDL.py below line 1371 (relevant part), and same file line 326 _NUMERIC_FIELDS = (first thing in class YoutubeDL(object): after the doc string, but this part should not have to be affected).

I might even do the refactor, but I fear my effort will be met with the same attitude. My refactor would be aimed at:

• Make _NUMERIC_FIELDS be collected from the authoritative locations, rather than repeating all the values here. My patch should only have to affect what I called "relevant part".
• In the relevant part, shorten the too-often repeated expression formats_info[0]. Not sure whether python will optimize this; if not, using a temporary variable would even give superficial performance gain in addition to making the code easier to maintain.
• Extract the relevant part into a separate file, so git can help maintainers more in merging far-diverged branches. In my experience, in a git workflow, big line numbers like this are a warning sign in themselves.
  • If for some reason someone here really likes big source files, we should at least put it into a function or something that makes it easy and obvious to denote this dictionary by a place name, rather than by line number.

[mk-pmb]

Once we can agree on structuring the code base in a way that makes additions like this one trivial, we should also consider how we can extend this easy-to-maintain extensibility to documentation.

Documentation for my suggested criteria would have to affect at least part of the "FORMAT SELECTION" in README.md, more precisely the list starting (in current master) line 701. Again, having to use line numbers to denote this location seems to be a code smell to me.

In an ideal world, my patch could deliver code and docs in close proximity, to increase chances that both will be kept in sync and the patch for both will be compatoble even with far-distant versions.

[mk-pmb]

I'll save us the effort of discussing whether the addition would be valid even in absence of more examples.

If you like my refactor idea anyway, I might still invest the effort, just so I can provide my own patch for those who see the holy light.

[mk-pmb]

At least in my own projects, I strive to make it easy for others (and future me) to extend and reuse my code, independent of whether I understand or approve a particular usecase at the time.

In my experience, my attempt to make my code easy to maintain (e.g. git friendly and having code locations easy to describe and locate accross versions) often helped make my own new patches trivial to apply even if a branch had been stale on a side track for years.

[mk-pmb]

I don't know what the hell you're talking about.

Imagine there could be, in the future, any derived numeric criterion that future-you might agree would be a good idea to support. (Maybe aspect ratio?)
Currently, your patch would have to affect at least three different locations. I even needed four, maybe due to lacking familiarity with the code base.

• One of them is the place where you'd actually implement the new formula to calculate the derived number.
• Two (for me: three) of them are a needless burden to maintainers, because they repeat information that's already present elsewhere in the code.
• Additionally, those two (for me: three) needless burdens are written in a way that provokes edit conflicts that git cannot automatically help you merge, albeit it could be written in a git friendly way.

Thus, the best patch I can currently come up with that would work without said refactoring, would be this one.

In an ideal world, my patch would instead have been 2 lines of python formulas and two lines of documentation about those formulas.

[mk-pmb]

Oh. Maybe we had a big misunderstanding. Is your point about more examples of portrait videos on YouTube? Or do you need proof that nowadays it's a commonplace phenomenon accross several video platforms? I had considered the latter universal knowledge.

[mk-pmb]

I see. I did not intend to force devs to hunt for example URLs.

I had imagined that we have some code module that calculates derived criteria based on extractor results. Such a module can first be tested with example data from existing real video tests, then tested with invented data or modified versions of the previous tests' input data, in this case, with swapped height/width.

In case of this feature request here, the latter approach would not require additional URLs and would thus be robust against breakage of those URLs.

[mk-pmb]

In this case, it would need to be at least two URLs, one landscape and one portrait, because of the nature of the request.

I provided them here, earlier:

• Portrait video: https://youtu.be/Y5iif7YskU4
• Landscape video: https://youtu.be/fCDsn7OTNMg

More examples would be ideal, but something is better than nothing.

At least for YouTube, I can provide some more already:

• Horizontal videos (currently 56 examples)
• Vertical videos (currently 24 examples)

Would you like even more YouTube examples?
Do you strictly require examples from other video platforms?

[mk-pmb]

So it seems you have still yet to provide a single example to justify making this change.

Could you elaborate what features the potential new examples should have that these don't? Do you mean I should provide combinations of video URLs where a single qualityLabel would not fit all of them?

Remember I explained above how this feature could be tested independent of any real, and thus potentially-breaking, URL examples. I might still continue to play the bureaucracy game as long as it leads to useful results, e.g. in the previous case, a tool to find vertical videos on YouTube. Someone might someday use it for something actually productive.

One problem with qualityLabels is that it doesn't convey the actual intent. Configuration that forces users to write stuff far from what they actually meant, is a common source of latent bugs.
From "360p" it is not apparent what limits should be used on the long dimension.

[mk-pmb]

Thank you for the clarification. Now that I know what kind of examples you're looking for, I can search for them.

Do we have documentation on how to use qualityLabel?
I tried, in the repo root, git grep -ni qualityLabel, but it only found one match, in the YouTube extractor.

I tried to guess the correct invocation, but both --format=360p and --format='qualityLabel=360p' failed for https://youtu.be/Mk0FyZqNp5Q .
For comparison, with my patch and --format='best[longside<=720][shortside<=360]', the correct video was downloaded.
Once I understand how to use qualityLabel, shall I make a PR to explain it in readme?

I don't currently have a way to search for tiny portrait videos on YouTube, but I'm willing to construct pathological cases using Twitter videos. Once I know how your qualityLabel idea is meant to be used, I can probably try it on this and this video.

Say we do merge this patch

My current patch is not intended for merge into the main project. It obvously lacks tests, and I had to use bad coding style to make it change as few lines as possible.

which is currently what you seem to be suggesting.

Then we had a misunderstanding. My current aim here is twofold:

• Originally, to allow easier configuration. However, currently, the code base is not fit for implementing this.
• As an intermediate goal, to find agreement on refactoring the code in a way that makes it easier to maintain. This also happens to make it easier to add format selection criteria derived by calculation from other values.

[mk-pmb]

Those both appear to be portrait videos, so you still don't seem to grasp the request at hand.

You seem to have missed the condition at the start of my sentence with the twitter links: "Once I know […]"
Unfortunately, the condition is still unmet, but nonetheless here's my updated…

Use case: Download the videos…

• … in formats where
  • one side is at most 360 px
  • and no side is larger than 720 px.
• … from these URLs:
  • https://youtu.be/Y5iif7YskU4
  • https://youtu.be/fCDsn7OTNMg
  • https://youtu.be/Mk0FyZqNp5Q
  • https://twitter.com/fullydecarbed/status/1493489050103255040
  • https://twitter.com/openreelvideo/status/1483109427175698438
  • https://twitter.com/roberticu/status/1361250232219168770
  • https://twitter.com/o_franco_aleman/status/1286275985697013760
• … with all required options stored in the config file
  • in such a way that maintaining the config is easy for the user.
• … with an easy-to-remember command line incantation.

With my requested feature, the required options in the config file would be:

• --format=mp4[longside<=720][shortside<=360]

And the command would be:

ytdl \
  https://youtu.be/Y5iif7YskU4 \
  https://youtu.be/fCDsn7OTNMg \
  https://youtu.be/Mk0FyZqNp5Q \
  https://twitter.com/fullydecarbed/status/1493489050103255040 \
  https://twitter.com/openreelvideo/status/1483109427175698438 \
  https://twitter.com/roberticu/status/1361250232219168770 \
  https://twitter.com/o_franco_aleman/status/1286275985697013760

Using my patch (still not meant to be merged because it's a bad solution), this exact command downloaded 7 videos.

Whether a solution with qualityLabel could work similarly easily, we'll probably only find out once it's implemented in the format selector. In an ideal world, adding the qualityLabel criterion would have been a one-line addition to the format selector code. (Plus one line of documentation.) In that ideal world, I'd probably even have done added it, just to see if it will work with the Twitter links.

[mk-pmb]

Since all the posts with objections seem to have magically vanished, I guess I can re-open the issue.

[dirkf]

I can see how these could be useful, more than a portrait/landscape flag.

See PR #30742.

[mk-pmb]

I can see how these could be useful,

Thank you. Let's hope someone will submit a clean and elegant implementation.
(Anyone reading this thread who can do it: Please do.)

[pukkandan]

Not trying to argue against addition of these fields, but you could just do:

• best[shortside>720] => best[height>720][width>720]
• best[shortside<720] => best[height<720][width>720]/best[width<720]
• best[longside>720] => best[height>720][width<720]/best[width>720]
• best[longside<720] => best[height<720][width<720]

[mk-pmb]

@pukkandan Thank you for your suggestion. Indeed my first local stopgap, before I made the above patch, was a script that took the meaningful expression and calculated all the combinatorial explosion required. This combinatorial explosion is the reason why in my later use case I added the criterion "in such a way that maintaining the config is easy for the user."

[mk-pmb]

I'm trying to write tests for this feature, so we can evaluate potential implementations. In my current attempt, to learn how to use our test environment, I started with adding trivial tests that current master should easily pass, using just height and width as size criteria. However,

• the "best" criterion seems to be strictly required, or no formats will be found. Is this the way it should be?
• somehow "best" tends to prefer square formats, even if they are smaller. Why?

[pukkandan]

You are not letting youtube-dl sort the formats. See

youtube-dl/test/test_YoutubeDL.py

Lines 59 to 64 in 6508688

	info_dict = _make_result(formats)
	yie = YoutubeIE(ydl)
	yie._sort_formats(info_dict['formats'])
	ydl.process_ie_result(info_dict)
	downloaded = ydl.downloaded_info_dicts[0]
	self.assertEqual(downloaded['ext'], 'webm')

and adjacent code

[mk-pmb]

@pukkandan Indeed, that fixed it. Thanks you!

[mk-pmb]

The sorting caues really strange behavior. It seems like it would sort strictly by height, rather than by area size / total pixel count.

[pukkandan]

All formats of a video should have same aspect ratio, so that is fine.

Unless an extractor indicates otherwise, the sorting is done based on these criteria (in order):

youtube-dl/youtube_dl/extractor/common.py

Lines 1428 to 1445 in 6508688

	return (
	preference,
	f.get('language_preference') if f.get('language_preference') is not None else -1,
	f.get('quality') if f.get('quality') is not None else -1,
	f.get('tbr') if f.get('tbr') is not None else -1,
	f.get('filesize') if f.get('filesize') is not None else -1,
	f.get('vbr') if f.get('vbr') is not None else -1,
	f.get('height') if f.get('height') is not None else -1,
	f.get('width') if f.get('width') is not None else -1,
	proto_preference,
	ext_preference,
	f.get('abr') if f.get('abr') is not None else -1,
	audio_ext_preference,
	f.get('fps') if f.get('fps') is not None else -1,
	f.get('filesize_approx') if f.get('filesize_approx') is not None else -1,
	f.get('source_preference') if f.get('source_preference') is not None else -1,
	f.get('format_id') if f.get('format_id') is not None else '',
	)

[pukkandan]

Indeed my first local stopgap, before I made the above patch, was a script that took the meaningful expression and calculated all the combinatorial explosion required. This combinatorial explosion is the reason why in my later use case I added the criterion "in such a way that maintaining the config is easy for the user."

Before yt-dlp (more specifically #25959), I used to have a script to generate -f strings too 😄

The "combinatorial explosion required" is a fundamental flaw with youtube-dl's format filtering options. Adding new fields will only solve your very specific issue (and only partially?), while others still have to resort to gigantic format strings (#26735, #26444 etc)

---

btw, instead of in process_video_result, it is better to add any "computed" fields here imo:

youtube-dl/youtube_dl/YoutubeDL.py

Lines 1612 to 1633 in 6508688

	for i, format in enumerate(formats):
	if format.get('format') is None:
	format['format'] = '{id} - {res}{note}'.format(
	id=format['format_id'],
	res=self.format_resolution(format),
	note=' ({0})'.format(format['format_note']) if format.get('format_note') is not None else '',
	)
	# Automatically determine file extension if missing
	if format.get('ext') is None:
	format['ext'] = determine_ext(format['url']).lower()
	# Automatically determine protocol if missing (useful for format
	# selection purposes)
	if format.get('protocol') is None:
	format['protocol'] = determine_protocol(format)
	# Add HTTP headers, so that external programs can use them from the
	# json output
	full_format_info = info_dict.copy()
	full_format_info.update(format)
	format['http_headers'] = self._calc_headers(full_format_info)
	# Remove private housekeeping stuff
	if '__x_forwarded_for_ip' in info_dict:
	del info_dict['__x_forwarded_for_ip']

[mk-pmb]

Unless an extractor indicates otherwise, the sorting is done based on these criteria (in order):
f.get('height') if f.get('height') is not None else -1,
f.get('width') if f.get('width') is not None else -1,

So my intuition was right.

All formats of a video should have same aspect ratio, so that is fine.

With "should have", do you mean that all currently supported extractors can never return results with mixed aspect ratio?
Is this claim backed by tests?

In my world, the purpose of the tests is to detect bugs early. Ideally, before they clash with a harsh reality. (In this case, it would be when a video provider starts adding cut-off square video.) Shouldn't we take this opportunity to make it prefer higher total pixel count?

[fstirlitz]

@mk-pmb Next time Steven Penny bothers you, just block him. Though if you choose to reply to him anyway, you may want to quote him in full, in case he deletes his own comments later. He does that a lot.

(I am not sure why @dirkf chooses to tolerate this missing stair instead of banning him from the repo, but whatever, it’s not my decision to make.)

For posterity (extracted from the GitHub API: <https://api.github.com/users/89z/events>)

---

@89z at 2022-03-13T01:29:44Z:

please provide example URLs demonstrating the problem

---

@89z at 2022-03-13T15:34:48Z:

Both of those have a qualityLabel of 360p, so cant you just select by that? Sorry I am not too familiar with YouTube-DL selectors, but that would be the right way to solve this I think.

---

@89z at 2022-03-13T15:46:22Z:

In general, I don't think its a good use of developer time to solve imagined issues, only real issues. So if you have a another example to present, then we can work toward a solution. Otherwise, I think this issue should be marked resolved.

---

@89z at 2022-03-13T16:43:59Z:

At first I wondered why such a simple addition warrants that much resistance. So I tried to make my own patch and share it here, for the benefit of all who value good UID.

Youve only provided one example. Based on that sample set alone, your solution is not the best one, using qualityLabel is.

Now if you have further examples, that could demonstrate a shortcoming of qualityLabel versus some other solution. But in absence of those examples, I maintain that your solution is worse, and I further maintain that the issue should be closed, as it seems a good solution is already available.

The fact that youre unwilling or unable to provide even a single example where qualityLabel wont work, reflects on your, rather than reflecting on those offering "resistance".

---

@89z at 2022-03-13T16:55:06Z:

While were at it, maybe we should add email [1] too?

1. https://wikipedia.org/wiki/Jamie_Zawinski#Zawinski's_Law

---

@89z at 2022-03-13T17:34:27Z:

I don't know what the hell you're talking about.

---

@89z at 2022-03-13T18:42:00Z:

Thats all well and good, but all your comments, and all your machinations have still yet to resolve the key problem with this entire issue. Even if the change was small, and even if you wrote the most elegant code, the code should be justified. Otherwise, I could post a new issue asking to add a line like this:

print('fart poop burp')

Like your patch, my patch here is small and concise. Do you see the problem? So again, unless you can justify why something like this is needed, other than with a purely theoretic argument, I maintain that the issue should remain closed. Two URLs would probably be enough; one landscape and one portrait, but it depends on the situation.

---

@89z at 2022-03-13T18:50:31Z:

If you wish to make any change at all, whether it is a bug fix or feature request, you should be proving an example URL. In this case, it would need to be at least two URLs, one landscape and one portrait, because of the nature of the request. More examples would be ideal, but something is better than nothing.

It is the responsibility of the poster to provide examples. You are the one asking for a change, so to force the developers to go hunting for their own examples, is frankly rude. Regardless of how common some situation might be, its still on the issue poster to demonstrate the issue with real examples.

---

@89z at 2022-03-13T19:01:09Z:

Since you seem unable to justify the existence of this change, I maintain that this issue is resolved and should remain closed.

---

@89z at 2022-03-13T22:04:31Z:

As previously discussed, qualityLabel is the better option for YouTube. So it seems you have still yet to provide a single example to justify making this change. If you can provide at least two URLs, one landscape and one portrait, that would be a start in finding a solution to this issue.

---

@89z at 2022-03-13T22:44:57Z:

Could you elaborate what features the potential new examples should have that these don't?

You haven't provided a single example, that is not better served by some other option. Every single example you've provided so far, would be better served by using qualityLabel as a selector. If you happen to find two YouTube videos that wouldn't work with the qualityLabel option (one landscape one portrait), then please link those. Otherwise it seems you will be forced to find links from another domain.

Do you mean I should provide combinations of video URLs where a single qualityLabel would not fit all of them?

I think you're making this way harder than it needs to be. Literally I am only asking for two URLs here. Most likely that cannot be YouTube URLs. If you are able to find some YouTube examples to serve as a pathological case, then that's fine.

Remember I explained above how this feature could be tested independent of any real, and thus potentially-breaking, URL examples.

Remember I explained that in general, its not a good use of developer time to work on imagined problems, but instead real problems. And without a concrete example of the issue, imagination is what we have.

I might still continue to play the bureaucracy game as long as it leads to useful results, e.g. in the previous case, a tool to find vertical videos on YouTube.

Continue? You have yet to start. You have yet to provide a single example that not better served by existing options, such as qualityLabel. I don't think it much to ask for a single example pair of URLs.

Someone might someday use it for something actually productive.

Here's the problem. Say we do merge this patch without any justification, which is currently what you seem to be suggesting. Five years from now, when another maintainer is working the codebase, someone reports an issue with the code you wrote. The maintainer looks around and around, and finally comes to this issue. Then the maintainer discovers that we never had any justification for the issue, we merged it "just because". If you want to fix a bug or add a feature, I am totally fine with that. But you have to understand, we have a single developer maintaining this project. So the responsibility is on the issue poster, to provide a justification for a change. Despite an original post and ten follow up comments, you have yet to do that. I am kind of impressed actually at your failure to do so.

---

@89z at 2022-03-13T23:36:55Z:

Do we have documentation on how to use qualityLabel?

I don't know if qualityLabel has even been implemented. My point is, and always has been, that using qualityLabel would be a better option than whats being suggested here. Or assuming its not implemented, then implementing qualityLabel would be better than implementing whats suggested here. qualityLabel is a JSON field directly from the response, so the logic could be much simpler than longside etc.

I don't currently have a way to search for tiny portrait videos on YouTube

See, this is exactly part of the issue. You thought up some imagined scenario, instead of encountering a scenario, and posting an issue in response to that. If you had just encountered some issue naturally, you would already have an example, and wouldn't need to go hunting for one.

but I'm willing to construct pathological cases using Twitter videos. Once I know how your qualityLabel idea is meant to be used, I can probably try it on this and this video.

Those both appear to be portrait videos, so you still don't seem to grasp the request at hand.

[dirkf]

I am not sure why @dirkf chooses to tolerate this missing stair instead of banning him from the repo, but whatever, it’s not my decision to make

Despite the ephemeral posting, his contributions are sometimes worthwhile.

Who can say what prompted the tendentious tirade above?

[mk-pmb]

@fstirlitz
Thank you for your gesture and advice. I applaud the recovery of the other side's arguments. As for archiving the author name, I have no strong opinion on that.

@dirkf

Who can say what prompted the tendentious tirade above?

They seemed to see both proposals as different and competing against each other for scarce developer time. Thus it makes sense that the posts disappeared exactly after I pointed out that the road to implementing both features is mostly the same.