search

senko / python-video-converter

Python Video Converter (ffmpeg wrapper)
469 stars 190 forks source link

Grab stream bitrate #17

Closed tahajahangir closed 11 years ago

tahajahangir commented 11 years ago

Currently, sound/video bitrate is not included in probe results. (only total bitrate is included)

Kami commented 11 years ago

Missing a test case, otherwise LGTM

tahajahangir commented 11 years ago

In think the best way to test this new feature to change MediaStreamInfo.__repr__ and also change the test_ffmpeg_probe (not extend it). Do you agree?

Kami commented 11 years ago

@tahajahangir Yes - you can add extra assertions to test_ffmpeg_probe, thanks!

senko commented 11 years ago

Looks good to me, yeah please just add the tests.

Regarding testing via repr(), it's not guaranteed that its output won't change in the future (the idea is to make it as useful as possible, but the output itsn't stable API). So while testing that repr is useful is good, I'd prefer explicit asserts for bitrate as well (in addition to updating the repr() test if needed).

tahajahangir commented 11 years ago

Test added (both explicit assert, and repr change)

senko commented 11 years ago

Awesome, thanks!