From 004b9d69fda1caaf7d383f5e88627e5c68d55c3e Mon Sep 17 00:00:00 2001 From: rohks Date: Tue, 30 Jul 2024 05:26:33 -0700 Subject: [PATCH] Handle case where length is unset in `FileDescriptorDataSource` - Modified the logic of `open()` and `read()` methods to handle scenarios where length is unset for the `FileDescriptor` provided. - Added unit test and contract test to handle this case. Also used `getDeclaredLength()` instead of `getLength()` to set the length of `AssetFileDescriptor` in unit tests and contract tests. PiperOrigin-RevId: 657551343 --- .../FileDescriptorDataSourceTest.java | 22 +++++- ...eUsingAssetFileDescriptorContractTest.java | 2 +- ...SourceUsingFileDescriptorContractTest.java | 10 ++- .../datasource/FileDescriptorDataSource.java | 78 ++++++++++++------- 4 files changed, 75 insertions(+), 37 deletions(-) diff --git a/libraries/datasource/src/androidTest/java/androidx/media3/datasource/FileDescriptorDataSourceTest.java b/libraries/datasource/src/androidTest/java/androidx/media3/datasource/FileDescriptorDataSourceTest.java index bd2cb181aa..184022954e 100644 --- a/libraries/datasource/src/androidTest/java/androidx/media3/datasource/FileDescriptorDataSourceTest.java +++ b/libraries/datasource/src/androidTest/java/androidx/media3/datasource/FileDescriptorDataSourceTest.java @@ -56,6 +56,20 @@ public final class FileDescriptorDataSourceTest { } } + @Test + public void testReadViaFileDescriptorWithUnsetLength() throws Exception { + File file = tempFolder.newFile(); + Files.write(Paths.get(file.getAbsolutePath()), DATA); + + try (FileInputStream inputStream = new FileInputStream(file)) { + DataSource dataSource = + new FileDescriptorDataSource(inputStream.getFD(), /* offset= */ 0, C.LENGTH_UNSET); + + TestUtil.assertDataSourceContent( + dataSource, new DataSpec(Uri.EMPTY), DATA, /* expectKnownLength= */ true); + } + } + @Test public void testReadViaFileDescriptorWithOffset() throws Exception { File file = tempFolder.newFile(); @@ -78,7 +92,7 @@ public final class FileDescriptorDataSourceTest { ApplicationProvider.getApplicationContext().getAssets().openFd(ASSET_PATH)) { DataSource dataSource = new FileDescriptorDataSource( - afd.getFileDescriptor(), afd.getStartOffset(), afd.getLength()); + afd.getFileDescriptor(), afd.getStartOffset(), afd.getDeclaredLength()); byte[] expectedData = TestUtil.getByteArray(ApplicationProvider.getApplicationContext(), ASSET_PATH); @@ -93,7 +107,7 @@ public final class FileDescriptorDataSourceTest { ApplicationProvider.getApplicationContext().getAssets().openFd(ASSET_PATH)) { DataSource dataSource = new FileDescriptorDataSource( - afd.getFileDescriptor(), afd.getStartOffset(), afd.getLength()); + afd.getFileDescriptor(), afd.getStartOffset(), afd.getDeclaredLength()); DataSpec dataSpec = new DataSpec(Uri.EMPTY, /* position= */ 100, C.LENGTH_UNSET); byte[] data = TestUtil.getByteArray(ApplicationProvider.getApplicationContext(), ASSET_PATH); byte[] expectedData = Arrays.copyOfRange(data, /* position= */ 100, data.length); @@ -110,11 +124,11 @@ public final class FileDescriptorDataSourceTest { DataSpec dataSpec = new DataSpec(Uri.EMPTY, /* position= */ 100, C.LENGTH_UNSET); DataSource dataSource1 = new FileDescriptorDataSource( - afd.getFileDescriptor(), afd.getStartOffset(), afd.getLength()); + afd.getFileDescriptor(), afd.getStartOffset(), afd.getDeclaredLength()); dataSource1.open(dataSpec); DataSource dataSource2 = new FileDescriptorDataSource( - afd.getFileDescriptor(), afd.getStartOffset(), afd.getLength()); + afd.getFileDescriptor(), afd.getStartOffset(), afd.getDeclaredLength()); // Opening a data source with the same file descriptor is expected to fail. assertThrows(DataSourceException.class, () -> dataSource2.open(dataSpec)); diff --git a/libraries/datasource/src/androidTest/java/androidx/media3/datasource/FileDescriptorDataSourceUsingAssetFileDescriptorContractTest.java b/libraries/datasource/src/androidTest/java/androidx/media3/datasource/FileDescriptorDataSourceUsingAssetFileDescriptorContractTest.java index e1cc93e1e0..217065dffc 100644 --- a/libraries/datasource/src/androidTest/java/androidx/media3/datasource/FileDescriptorDataSourceUsingAssetFileDescriptorContractTest.java +++ b/libraries/datasource/src/androidTest/java/androidx/media3/datasource/FileDescriptorDataSourceUsingAssetFileDescriptorContractTest.java @@ -41,7 +41,7 @@ public class FileDescriptorDataSourceUsingAssetFileDescriptorContractTest AssetFileDescriptor afd = ApplicationProvider.getApplicationContext().getAssets().openFd(ASSET_PATH); return new FileDescriptorDataSource( - afd.getFileDescriptor(), afd.getStartOffset(), afd.getLength()); + afd.getFileDescriptor(), afd.getStartOffset(), afd.getDeclaredLength()); } @Override diff --git a/libraries/datasource/src/androidTest/java/androidx/media3/datasource/FileDescriptorDataSourceUsingFileDescriptorContractTest.java b/libraries/datasource/src/androidTest/java/androidx/media3/datasource/FileDescriptorDataSourceUsingFileDescriptorContractTest.java index dfaca4dbad..6181ed70e1 100644 --- a/libraries/datasource/src/androidTest/java/androidx/media3/datasource/FileDescriptorDataSourceUsingFileDescriptorContractTest.java +++ b/libraries/datasource/src/androidTest/java/androidx/media3/datasource/FileDescriptorDataSourceUsingFileDescriptorContractTest.java @@ -16,6 +16,7 @@ package androidx.media3.datasource; import android.net.Uri; +import androidx.media3.common.C; import androidx.media3.test.utils.DataSourceContractTest; import androidx.media3.test.utils.TestUtil; import androidx.test.ext.junit.runners.AndroidJUnit4; @@ -26,6 +27,7 @@ import java.io.FileInputStream; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; +import java.util.List; import org.junit.After; import org.junit.Ignore; import org.junit.Rule; @@ -54,15 +56,17 @@ public class FileDescriptorDataSourceUsingFileDescriptorContractTest } @Override - protected DataSource createDataSource() throws Exception { + protected List createDataSources() throws Exception { File file = tempFolder.newFile(); Files.write(Paths.get(file.getAbsolutePath()), DATA); inputStream = new FileInputStream(file); - return new FileDescriptorDataSource(inputStream.getFD(), /* offset= */ 0, DATA.length); + return ImmutableList.of( + new FileDescriptorDataSource(inputStream.getFD(), /* offset= */ 0, DATA.length), + new FileDescriptorDataSource(inputStream.getFD(), /* offset= */ 0, C.LENGTH_UNSET)); } @Override - protected ImmutableList getTestResources() throws Exception { + protected ImmutableList getTestResources() { return ImmutableList.of( new TestResource.Builder() .setName("simple") diff --git a/libraries/datasource/src/main/java/androidx/media3/datasource/FileDescriptorDataSource.java b/libraries/datasource/src/main/java/androidx/media3/datasource/FileDescriptorDataSource.java index 883d8aafca..d97e1013b5 100644 --- a/libraries/datasource/src/main/java/androidx/media3/datasource/FileDescriptorDataSource.java +++ b/libraries/datasource/src/main/java/androidx/media3/datasource/FileDescriptorDataSource.java @@ -28,11 +28,11 @@ import androidx.media3.common.C; import androidx.media3.common.PlaybackException; import androidx.media3.common.util.UnstableApi; import com.google.common.collect.Sets; -import java.io.EOFException; import java.io.FileDescriptor; import java.io.FileInputStream; +import java.io.FileNotFoundException; import java.io.IOException; -import java.io.InputStream; +import java.nio.channels.FileChannel; import java.util.Set; /** @@ -58,7 +58,7 @@ public class FileDescriptorDataSource extends BaseDataSource { private final long length; @Nullable private Uri uri; - @Nullable private InputStream inputStream; + @Nullable private FileInputStream inputStream; private long bytesRemaining; private boolean opened; @@ -78,28 +78,57 @@ public class FileDescriptorDataSource extends BaseDataSource { @Override public long open(DataSpec dataSpec) throws DataSourceException { - if (!inUseFileDescriptors.add(fileDescriptor)) { + try { + uri = dataSpec.uri; + transferInitializing(dataSpec); + + if (!inUseFileDescriptors.add(fileDescriptor)) { + throw new DataSourceException( + new IllegalStateException("Attempted to re-use an already in-use file descriptor"), + PlaybackException.ERROR_CODE_INVALID_STATE); + } + + if (length != C.LENGTH_UNSET && dataSpec.position > length) { + throw new DataSourceException(PlaybackException.ERROR_CODE_IO_READ_POSITION_OUT_OF_RANGE); + } + + seekFileDescriptor(fileDescriptor, offset + dataSpec.position); + inputStream = new FileInputStream(fileDescriptor); + + if (length == C.LENGTH_UNSET) { + // The asset must extend to the end of the file. We can try and resolve the length with + // FileInputStream.getChannel().size(). + FileChannel channel = inputStream.getChannel(); + long channelSize = channel.size(); + if (channelSize == 0) { + bytesRemaining = C.LENGTH_UNSET; + } else { + bytesRemaining = channelSize - channel.position(); + if (bytesRemaining < 0) { + // The seek above was successful, but the new position is beyond the end of the file. + throw new DataSourceException( + PlaybackException.ERROR_CODE_IO_READ_POSITION_OUT_OF_RANGE); + } + } + } else { + bytesRemaining = length - dataSpec.position; + if (bytesRemaining < 0) { + throw new DataSourceException(PlaybackException.ERROR_CODE_IO_READ_POSITION_OUT_OF_RANGE); + } + } + } catch (DataSourceException e) { + throw e; + } catch (IOException e) { throw new DataSourceException( - new IllegalStateException("Attempted to re-use an already in-use file descriptor"), - PlaybackException.ERROR_CODE_INVALID_STATE); + e, + e instanceof FileNotFoundException + ? PlaybackException.ERROR_CODE_IO_FILE_NOT_FOUND + : PlaybackException.ERROR_CODE_IO_UNSPECIFIED); } - if (dataSpec.position > length) { - throw new DataSourceException(PlaybackException.ERROR_CODE_IO_READ_POSITION_OUT_OF_RANGE); - } - - uri = dataSpec.uri; - transferInitializing(dataSpec); - seekFileDescriptor(fileDescriptor, offset + dataSpec.position); - inputStream = new FileInputStream(fileDescriptor); - if (dataSpec.length != C.LENGTH_UNSET) { bytesRemaining = - length != C.LENGTH_UNSET - ? min(dataSpec.length, length - dataSpec.position) - : dataSpec.length; - } else { - bytesRemaining = length != C.LENGTH_UNSET ? length - dataSpec.position : C.LENGTH_UNSET; + bytesRemaining == C.LENGTH_UNSET ? dataSpec.length : min(bytesRemaining, dataSpec.length); } opened = true; @@ -123,15 +152,6 @@ public class FileDescriptorDataSource extends BaseDataSource { throw new DataSourceException(e, PlaybackException.ERROR_CODE_IO_UNSPECIFIED); } if (bytesRead == -1) { - if (bytesRemaining != C.LENGTH_UNSET) { - throw new DataSourceException( - new EOFException( - String.format( - "Attempted to read %d bytes starting at position %d, but reached end of the" - + " file before reading sufficient data.", - this.length, this.offset)), - PlaybackException.ERROR_CODE_IO_UNSPECIFIED); - } return C.RESULT_END_OF_INPUT; } if (bytesRemaining != C.LENGTH_UNSET) {