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.

Sign up for GitHub

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

Jump to bottom

Invalid MPD manifest #49

Closed
lpluto opened this issue Nov 16, 2015 · 7 comments
Closed

Invalid MPD manifest #49

lpluto opened this issue Nov 16, 2015 · 7 comments
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly

Comments

@lpluto
Copy link

lpluto commented Nov 16, 2015

Hi,
Sometimes we receive invalid output MPD manifest when we use "--mpd_output" option. We guess that it may be related to wrong threads synchronization when edash packager writes to output MPD file.

It doesn't happen when we use mpd_generator command.

Sample invalid manifest: manifest.mpd.txt

Kind regards,
Lukasz Pluto
@kqyang
Copy link
Contributor

kqyang commented Nov 16, 2015

No, the thread synchronization issue in #45 should not lead to corrupted mpd output. I don't see how it could happen. @rkuroiwa what do you think?

Lukasz, do you have sample files and a duplicate procedure we can use to duplicate the problem?

@kqyang kqyang closed this as completed Nov 16, 2015
@kqyang
Copy link
Contributor

kqyang commented Nov 16, 2015

Oops, click the button by mistake

@kqyang kqyang reopened this Nov 16, 2015
@rkuroiwa
Copy link

I think the problem is a race condition in MpdNotifier::Flush() implementations. There are no locks used in Flush() so multiple threads are writing to the same file.
There will be a patch soon that locks the Flush() implementations.

That said, it would be nice to know the repro steps :)

rkuroiwa pushed a commit that referenced this issue Nov 16, 2015
Remove locks from MpdBuilder and lock at MpdNotifier level
981041a 
- Since most use cases use notifier implementations that already uses
  locks, removing locks from MpdBuilder.
- Added locks to MpdNotifier::Flush() implementations. Multiple threads
  were able to write to the same file.

Issue #45, #49

Change-Id: I6e39824485672f40e6c947da97f1743fac174167
@kqyang kqyang added the type: bug Something isn't working correctly label Nov 16, 2015
@kqyang
Copy link
Contributor

kqyang commented Nov 16, 2015

This bug should be fixed with Rintaro's latest cl.

@lpluto Can you try and verify? Let us know the results. Thanks again for reporting the issue!

rkuroiwa pushed a commit that referenced this issue Nov 19, 2015
@kqyang
Remove locks from MpdBuilder and lock at MpdNotifier level
3615e0e 
- Since most use cases use notifier implementations that already uses
  locks, removing locks from MpdBuilder.
- Added locks to MpdNotifier::Flush() implementations. Multiple threads
  were able to write to the same file.

Issue #45, #49

Change-Id: I6e39824485672f40e6c947da97f1743fac174167
@kqyang
Copy link
Contributor

kqyang commented Nov 19, 2015

Close as fixed. @lpluto please reopen it if you are still seeing the problem with latest cl or v1.2.1.

@kqyang kqyang closed this as completed Nov 19, 2015
@lpluto
Copy link
Author

lpluto commented Nov 20, 2015

We've updated eDash Packager on one of our environments and it works now.

Thanks for help!

@kqyang
Copy link
Contributor

kqyang commented Nov 20, 2015

Great to hear that! Thanks for the update!

@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 19, 2018
@shaka-project shaka-project locked and limited conversation to collaborators Apr 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@kqyang @rkuroiwa @lpluto @shaka-bot