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

Fix an exception that is thrown when the manifest is valid but does not contain trackId data #248

Merged
merged 1 commit into from
May 29, 2024
Merged

Fix an exception that is thrown when the manifest is valid but does not contain trackId data #248

merged 1 commit into from
May 29, 2024

Conversation

schuettecarsten
Copy link
Contributor

We have several assets in our media services that have a ISM file like this:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<smil xmlns="http://www.w3.org/2001/SMIL20/Language">
    <head>
        <meta content="mp4" name="formats"/>
    </head>
    <body>
        <switch>
            <video src="file_1000_640x360_1000.mp4"/>
            <audio src="file_1000_640x360_1000.mp4" systemBitrate="128000" title="AAC_eng_ch2_128kbps"/>
        </switch>
    </body>
</smil>

When processing this asset using the migration tool, an exception is thrown:

2024-05-29 13:17:35.605 +00:00 [ERR] Failed to migrate asset 18c23a5d-1500-80c4-015f-f1e577fcccbc Error:System.ArgumentNullException: Value cannot be null. (Parameter 'source')
   at System.Linq.ThrowHelper.ThrowArgumentNullException(ExceptionArgument argument)
   at System.Linq.Enumerable.TryGetSingle[TSource](IEnumerable`1 source, Func`2 predicate, Boolean& found)
   at System.Linq.Enumerable.Single[TSource](IEnumerable`1 source, Func`2 predicate)
   at AMSMigrate.Contracts.Track.get_TrackID() in S:\Tools\Microsoft\azure-media-migration\migrationTool\contracts\Manifest.cs:line 54
   at AMSMigrate.Transform.ShakaPackager.<>c__DisplayClass6_0.<GetArguments>b__0(Track t, Int32 i) in S:\Tools\Microsoft\azure-media-migration\migrationTool\transform\ShakaPackager.cs:line 142
   at System.Linq.Enumerable.SelectIterator[TSource,TResult](IEnumerable`1 source, Func`3 selector)+MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at AMSMigrate.Transform.ShakaPackager.GetArguments(IList`1 inputs, IList`1 outputs, IList`1 manifests) in S:\Tools\Microsoft\azure-media-migration\migrationTool\transform\ShakaPackager.cs:line 127
   at AMSMigrate.Transform.ShakaPackager.RunAsync(String workingDirectory, String[] inputs, String[] outputs, String[] manifests, CancellationToken cancellationToken) in S:\Tools\Microsoft\azure-media-migration\migrationTool\transform\ShakaPackager.cs:line 297
   at AMSMigrate.Transform.PackageTransform.TransformAsync(AssetDetails details, ValueTuple`2 outputPath, CancellationToken cancellationToken) in S:\Tools\Microsoft\azure-media-migration\migrationTool\transform\PackageTransform.cs:line 161

This PR fixes this exception by adding a default value for TrackID.

duggaraju
duggaraju reviewed May 29, 2024
View reviewed changes
Copy link
Collaborator

@duggaraju duggaraju left a comment

Choose a reason for hiding this comment

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

Do we need Parameters?. since the Parameters is never null (It is an empty Array by default)?

duggaraju
duggaraju reviewed May 29, 2024
View reviewed changes
migrationTool/contracts/Manifest.cs
@@ -51,9 +51,9 @@ public Track(StreamType type)
// Check if the track is stored as one file per fragment.
public bool IsMultiFile => string.IsNullOrEmpty(Path.GetExtension(Source));

public uint TrackID => uint.Parse(Parameters.Single(p => p.Name == "trackID").Value);
public uint TrackID => uint.Parse(Parameters?.SingleOrDefault(p => p.Name == "trackID")?.Value ?? "1");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Parameters should never be null (Initialized to empty array) so the first ? is not needed. But parameters themselves are optional so the second one is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, Parameters should not be null because they have a default initializer, but they can be assigned to null using the setter. I just wanted to make sure to catch this also.

@pohhsu pohhsu merged commit 8caf933 into Azure:main May 29, 2024
4 checks passed
@schuettecarsten schuettecarsten deleted the pr/fix-track-id-exception branch May 31, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@duggaraju duggaraju duggaraju approved these changes

@pohhsu pohhsu pohhsu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@schuettecarsten @duggaraju @pohhsu