Skip to content

fix: Remove EXT-X-ENDLIST from master playlist file #519

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

Merged
merged 4 commits into from
Apr 1, 2025

Conversation

dave-nicholas
Copy link
Contributor

@dave-nicholas dave-nicholas commented Feb 4, 2024

I am a contributor on the shaka project and noticed that playback was broken with this library.
This PR fixes a bug in the master playlist and allows playback with shaka (and probably other players where playback previously failed).

Unlike the EXTM3U element which is required at the start every Media Playlist and every Master Playlist. The EXT-X-ENDLIST should only appear in a Media Playlist file.

@exxsy
Copy link

exxsy commented Jun 17, 2024

Yeah EXT-X-ENDLIST line causes shaka player to throw error

@exxsy
Copy link

exxsy commented Jun 17, 2024

If you just dont want to wait PR,

Create a PlaylistGenerator for this case, like this one.

namespace App\PlaylistGenerators;

class HLSExtendedGenerator implements PlaylistGenerator
{
    public const PLAYLIST_START = '#EXTM3U';

    private function getStreamInfoLine(Media $segmentPlaylistMedia, string $key): string
    {
        $segmentPlaylist = $segmentPlaylistMedia->getDisk()->get(
            $segmentPlaylistMedia->getDirectory() . HLSExporter::generateTemporarySegmentPlaylistFilename($key)
        );

        $lines = DynamicHLSPlaylist::parseLines($segmentPlaylist)->filter();

        return $lines->get($lines->search($segmentPlaylistMedia->getFilename()) - 1);
    }

    public function get(array $segmentPlaylists, PHPFFMpeg $driver): string
    {
        return Collection::make($segmentPlaylists)->map(function (Media $segmentPlaylist, $key) use ($driver) {
            $streamInfoLine = $this->getStreamInfoLine($segmentPlaylist, $key);

            $media = (new MediaOpener($segmentPlaylist->getDisk(), $driver))
                ->openWithInputOptions($segmentPlaylist->getPath(), ['-allowed_extensions', 'ALL']);

            if ($media->getVideoStream()) {
                if ($frameRate = StreamParser::new($media->getVideoStream())->getFrameRate()) {
                    $streamInfoLine .= ",FRAME-RATE={$frameRate}";
                }
            }

            return [$streamInfoLine, $segmentPlaylist->getFilename()];
        })->collapse()->prepend(self::PLAYLIST_START)->implode(PHP_EOL);
    }
}

Then use it like this,

        FFMpeg::fromDisk("videos")
            ->open("/$this->name.$this->extension")
            ->exportForHLS()
            ->withPlaylistGenerator(new HLSExtendedGenerator)
            ->setSegmentLength(10)->...

@francoism90
Copy link

Could this please be merged? I've got the same issue on a new project, and using this package.

@exxsy Thanks for the workaround. It's basically a copy of the current one without the line finish. I would suggest also making those methods/vars protected instead of private. This would make it easier to simple extend the current one, and overrule what's needed. :)

@exxsy
Copy link

exxsy commented Jan 17, 2025

@francoism90 since it was a workaround and temporary, i didnt think that much. You're welcome. Happy coding

@francoism90
Copy link

@exxsy This was more a comment for upstream. :)

I'll try to make a PR with this change.

@nagi1
Copy link

nagi1 commented Mar 25, 2025

Hello, Thanks @protonemedia for this awesome package!
Any plans to merge this PR?

@nagi1
Copy link

nagi1 commented Mar 25, 2025

I created a fork with all open PRs merged until @pascalbaljet or @protonemedia merge them.
https://github.com/nagi1/laravel-ffmpeg

Thanks For this awesome package really appreciated!

@pascalbaljet
Copy link
Collaborator

I've made it optional because I find it too risky to remove it by default, as we have never had problems with it.

FFMpeg::fromDisk("videos")
    ->open("/$this->name.$this->extension")
    ->exportForHLS()
    ->withoutPlaylistEndLine()

@pascalbaljet pascalbaljet merged commit d1125f7 into protonemedia:main Apr 1, 2025
33 of 36 checks passed
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.

None yet

5 participants