dominic-p / videojs-resolution-selector

Adds a resolution selector button to Video.js to allow users to manually adjust the video quality.
MIT License
88 stars 31 forks source link

Switch order of available resolutions #31

Closed perttumyry closed 9 years ago

perttumyry commented 9 years ago

We noticed that when using multiple different types of sources the plugin stacks these types to array in reverse order.

Here's an example.

<source type="video/mp4" src="video-file-360.mp4" data-res="360">
<source type="video/mp4" src="video-file-480.mp4" data-res="480">
<source type="video/webm" src="video-file-360.webm" data-res="360">
<source type="video/webm" src="video-file-480.webm" data-res="480">

This ends up the be a following array in player.availableRes.

[
    {
        data-res: "360",
        src: "video-file-360.webm",
        type: "video/webm"
    },
    {
        data-res: "480",
        src: "video-file-480.webm",
        type: "video/webm"
    },
    {
        data-res: "360",
        src: "video-file-360.mp4",
        type: "video/mp4"
    },
    {
        data-res: "480",
        src: "video-file-480.mp4",
        type: "video/mp4"
    }
]

So the 360 and 480 orders are correct, but webm sources are placed before mp4 sources. This could be rather easily fixed by replacing available_res[current_res].push( sources[i] ); with available_res[current_res].unshift( sources[i] ); in video-quality-selector.js line 221. Do you think you could commit such a change to the project?

dominic-p commented 9 years ago

Wouldn't the output be the following?

{
    "360" : [
        {
            data-res: "360",
            src: "video-file-360.webm",
            type: "video/webm"
        },
        {
            data-res: "360",
            src: "video-file-360.mp4",
            type: "video/mp4"
        }
    ],
    "480" : [
        {
            data-res: "480",
            src: "video-file-480.webm",
            type: "video/webm"
        },
        {
            data-res: "480",
            src: "video-file-480.mp4",
            type: "video/mp4"
        }
    ]
}

Either way, you're right that we shouldn't mess with the order. I'll see about make that change.