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

Feat/multiples inputs #11

Closed
wants to merge 8 commits into from

Conversation

luiscovelo
Copy link
Collaborator

@luiscovelo luiscovelo commented Jan 14, 2024

Hi, @mauricioabreu.

I fix the command builder to mount dynamic video inputs;

I created a manifest handler to serve a hls player a custom result containing all mosaics from tasks.json;

I update hls player to show buttons options reference each mosaic;

See this video with hls player updated: HLS player working.

Best reguards.

@@ -10,7 +10,7 @@ import (
)

func Build(m mosaic.Mosaic, cfg *config.Config) []string {
playlistPath := fmt.Sprintf("hls/%s/playlist.m3u8", m.Name)
playlistPath := fmt.Sprintf("hls/%s/playlist-%s.m3u8", m.Name, m.Name)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why repeating the name of the mosaic again?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hls/mosaicname/playlist.m3u8

This is how the path looks like. It works for HLS.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make unique each mosaic configured in tasks.json, because if you create a new mosaic object and maintains a playlist.m3u8 name, the final result will be mixed into a unique HLS output, therefore into HLS player, I can to change the manifest.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I get what you are saying. But the answer for this problem is creating a new bucket. Every mosaic should have its own bucket, then, for example:

  • mosaic1 will have its files in mosaic1 bucket
  • mosaic2 will have its files in mosaic2 bucket

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure when the bucket should be created. Probably just before starting the ffmpeg process

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we can to create only one bucket, like hls or objects and use the prefixes to organize fragments, see how this works in AWS S3 article.
What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look this:
bucket with directories

Basically, join the folder name + filename and upload object:
image

This way we'll have one bucket organized by prefixes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks great!

@@ -40,14 +40,25 @@ func Build(m mosaic.Mosaic, cfg *config.Config) []string {

filterComplexBuilder.WriteString(fmt.Sprintf("[image]%s overlay=shortest=0 [mosaic]", lastOverlay))

videoInputs := []string{}
for _, m := range m.Medias {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

) {
lifecycle.Append(fx.Hook{
OnStart: func(ctx context.Context) error {
r := mux.NewRouter()
r.Handle("/playlist/{filename}", playlistHandler).Methods("GET")
r.Handle("/fragment/{filename}", fragmentHandler).Methods("GET")
r.Handle("/player", playerHandler).Methods("GET")
r.Handle("/manifest", manifestHandler).Methods("GET")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not understand the reason behind this endpoint, sorry. Could you explain?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I think we can use just one handler and pass the request path to the storage. What is your opinion?

Copy link
Collaborator Author

@luiscovelo luiscovelo Jan 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created this new endpoint to serve a custom json with all mosaics playlist.m3u8 and can possible to create a button for each mosaic registered:

Manifest response based in tasks.json:

GET http://localhost:8090/manifest

[
    {
        "name": "mosaic",
        "playlist_url": "http://localhost:8090/playlist/playlist-mosaic.m3u8"
    },
    {
        "name": "mosaic2",
        "playlist_url": "http://localhost:8090/playlist/playlist-mosaic2.m3u8"
    }
]

On HLS player, i can mount this:

Buttons to select a mosaic

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But, now i revised and HLS player use same endpoint to get playlist and fragment (/player/playlist/filename), then we need only two endpoints:

  • GET /playlist
  • GET /manifest

@mauricioabreu
Copy link
Collaborator

Could you split this PR into differents features/fixes? (not sure if that manifest change makes sense)

@luiscovelo
Copy link
Collaborator Author

luiscovelo commented Jan 14, 2024

Could you split this PR into differents features/fixes? (not sure if that manifest change makes sense)

Of course, i'll split this changes in another branchs to make easy a maintainability.

Related to manifest change, we can to discuss another ways to make this.

@mauricioabreu
Copy link
Collaborator

@luiscovelo given the last PRs, how many changes in this PR will be kept?
Do we still need this PR?

Thank you for your contributions!

@luiscovelo luiscovelo closed this Jan 14, 2024
@luiscovelo
Copy link
Collaborator Author

luiscovelo commented Jan 14, 2024

@luiscovelo given the last PRs, how many changes in this PR will be kept? Do we still need this PR?

Thank you for your contributions!

@mauricioabreu,

We don't need kept this PR, because i split into new PR's merged in branch main.

Related about manifest endpoint, with theses new changes applied into main, i can to create a new handler to return a custom object for HLS player, containing all mosaics playlist, based in tasks.json, like this:

[
    {
        "name": "mosaic",
        "playlist_url": "http://localhost:8090/playlist/playlist-mosaic.m3u8"
    },
    {
        "name": "mosaic2",
        "playlist_url": "http://localhost:8090/playlist/playlist-mosaic2.m3u8"
    }
]

And to mount a button for each mosaic returned.

Like this:

image

I guess creating this new endpoint makes it dynamic for HLS player.

What do you think?

@mauricioabreu
Copy link
Collaborator

It looks like a good idea. Would this feature be useful just for testing, right?

@luiscovelo
Copy link
Collaborator Author

@mauricioabreu,

I had thinking as a feature in case the user to have more than one mosaic registered.

How do you think to do so that the user can switch their mosaics for viewing?

@mauricioabreu
Copy link
Collaborator

@luiscovelo well, if someone/company is going to use this project, they will integrate in their own players.
This is why I think the player component acts like a testing component

@luiscovelo
Copy link
Collaborator Author

@mauricioabreu,

Of course, you are right, don't make a sense implement this endpoint, for testing, one mosaic entry will be enough.

@luiscovelo luiscovelo deleted the feat/multiples-inputs branch January 18, 2024 20:27
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