Ability of defining CustomMencoderOptions on renderer level #8

Merged
merged 2 commits into from Jan 12, 2012

Projects

None yet

4 participants

Contributor
ExSport commented Jan 12, 2012

Hi all
I added possibility to define CustomMencoderOptions on renderer level so we can now fix eg. bad AR on TV renderers when transcoding is used, upscaling or enabling deinterlacers differently for every renderer.
Also I added some info for hidden params of Renderer.conf
Please feel free to edit is(typos, missing info, bad english etc.)
Some discussion about this feature which is already implemented in SubJunk builds:
http://www.ps3mediaserver.org/forum/viewtopic.php?f=11&t=7930
Thx ExSport

Member

All pro. I've been asked to add this to pms-mlx too

Member

Nice addition.
I'll merge it.

@Raptor399 Raptor399 merged commit 9e6aad5 into ps3mediaserver:master Jan 12, 2012
Contributor
ExSport commented Jan 12, 2012

Great, thx!

Member

Oh, and @ExSport:

Next time you make a pull request, please create a separate branch for every pull request with exactly the code in it that you want us to pull.

I usually create a fresh branch based on the PMS upstream, then cherry-pick my commits onto that, then push that tree to my online repository and create a pull request from there.

Why? Follow up commits are automatically added to the pull request by GitHub.
So if you create a pull request from your own master, over time the pull request will change to more and more changes.

This is what I usually do:

# Only need to add the "upstream" remote repo once.
git remote add upstream git://github.com/ps3mediaserver/ps3mediaserver.git
git checkout -b mencoder-fix remotes/upstream/master
git cherry-pick e1b0139
git push origin mencoder-fix
# Ready to create the pull request!

Second side note: try to keep your commits clean and to the point.
The changed "Panasonic.conf" didn't really have anything to do with adding the ability to defining CustomMencoderOptions on the renderer level.

Contributor
ExSport commented Jan 12, 2012

Ok, quite new on github so thanks for info.
I agree with modifying Panasonic.conf but I did the change due to some bugs/obsolete things there but wasn't able to make Pull request without these modifications. With TortoiseSVN it was very easy to define from which modified files to create .patch file so I was able to exclude some modifs.
Need to read GitHub Help again and again to understand all things here.

Owner

This breaks global custom MEncoder options. Defining a property without a value:

CustomMencoderOptions =

returns an empty string, i.e. not null, so the global options are never used.

Owner

Fixed in fe41952.

Contributor

Good catch. Thanks!
Maybe I tested it with and without this property defined so didn't spotted this "empty string" bug:-(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment