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

bitmovin: bugfix for level manifests not being created in the right path (fixes #197) #198

Closed
wants to merge 1 commit into from

Conversation

flavioribeiro
Copy link
Member

@flavioribeiro flavioribeiro commented Sep 16, 2018

This pull request fixes the PATH for main and level manifests that were not pointing to the right ts files structure.

Bitmovin is still creating some other manifest files that are not being used/mapped on the master manifest. The final file structure after this fix looks like this:

image

I will defer to @jamescyeh and @fsouza on accepting this or not but will go ahead and use my fork in the mean time.

Cheers!

@fsouza fsouza self-requested a review September 16, 2018 23:59
@fsouza
Copy link
Collaborator

fsouza commented Sep 18, 2018

@flavioribeiro with this change, doesn't it drop all the files at the root of the bucket?

@flavioribeiro
Copy link
Member Author

bitmovin creates folders for each level with an ID so with this PR it looks like this:

image

Please note that we still have two problems with this structure:

  • We have duplicated level manifests (one with the id, another one with the name passed on job.json)
  • We are creating one audio output for each video bitrate -- we should possibly be detecting audios with same parameters and just reuse them. Better than that, we should also be able to mux the audio in the mpeg-ts segments but I'm not sure if the Bitmovin API allows us to do this.

@flavioribeiro
Copy link
Member Author

let's add @jamescyeh to the loop as I've talked to him offline about the issues.

@fsouza
Copy link
Collaborator

fsouza commented Sep 18, 2018

But Bitmovin is doing that because we're asking it to do it, no? I'd say the correct behavior would be something like this:

jobId
├── master_1080p
│   └── segment-00001.ts
├── master_240p
│   └── segment-00001.ts
├── master_360p
│   └── segment-00001.ts
├── master_480p
│   └── segment-00001.ts
├── master_720p
│   └── segment-00001.ts
├── master.m3u8
├── master_1080p.m3u8
├── master_240p.m3u8
├── master_360p.m3u8
├── master_480p.m3u8
└── master_720p.m3u8

What do you think?

@flavioribeiro
Copy link
Member Author

I'm not opinionated on the files/manifest structure as long as they work haha

the manifests could live inside the folders but whatever you guys decide would work for me as long as I get the master manifest back when I describe the job on getting /jobs/jobId endpoint (#196). I can also jump in and try to do something on that front too

@fsouza
Copy link
Collaborator

fsouza commented Sep 19, 2018

@flavioribeiro cool! I'll take a stab at this later today or tomorrow. I think that this PR as is would end-up with the manifest files at the rooot of the output bucket. Maybe not, dunno lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants