[pull] main from immich-app:main

server/src/repositories/asset.repository.ts

@@ -195,10 +195,10 @@ export class AssetRepository {
           .values(audio)
           .onConflict((oc) =>
             oc.column('assetId').doUpdateSet(({ ref }) => ({
-              bitrate: ref('asset_audio.bitrate'),
-              index: ref('asset_audio.index'),
-              profile: ref('asset_audio.profile'),
-              codecName: ref('asset_audio.codecName'),
+              bitrate: ref('excluded.bitrate'),
+              index: ref('excluded.index'),
+              profile: ref('excluded.profile'),
+              codecName: ref('excluded.codecName'),
             })),
           ),
       );
@@ -211,21 +211,22 @@ export class AssetRepository {
           .values(video)
           .onConflict((oc) =>
             oc.column('assetId').doUpdateSet(({ ref }) => ({
-              bitrate: ref('asset_video.bitrate'),
-              timeBase: ref('asset_video.timeBase'),
-              index: ref('asset_video.index'),
-              profile: ref('asset_video.profile'),
-              level: ref('asset_video.level'),
-              colorPrimaries: ref('asset_video.colorPrimaries'),
-              colorTransfer: ref('asset_video.colorTransfer'),
-              colorMatrix: ref('asset_video.colorMatrix'),
-              dvProfile: ref('asset_video.dvProfile'),
-              dvLevel: ref('asset_video.dvLevel'),
-              dvBlSignalCompatibilityId: ref('asset_video.dvBlSignalCompatibilityId'),
-              codecName: ref('asset_video.codecName'),
-              formatName: ref('asset_video.formatName'),
-              formatLongName: ref('asset_video.formatLongName'),
-              pixelFormat: ref('asset_video.pixelFormat'),
+              bitrate: ref('excluded.bitrate'),
+              frameCount: ref('excluded.frameCount'),
+              timeBase: ref('excluded.timeBase'),
+              index: ref('excluded.index'),
+              profile: ref('excluded.profile'),
+              level: ref('excluded.level'),
+              colorPrimaries: ref('excluded.colorPrimaries'),
+              colorTransfer: ref('excluded.colorTransfer'),
+              colorMatrix: ref('excluded.colorMatrix'),
+              dvProfile: ref('excluded.dvProfile'),
+              dvLevel: ref('excluded.dvLevel'),
+              dvBlSignalCompatibilityId: ref('excluded.dvBlSignalCompatibilityId'),
+              codecName: ref('excluded.codecName'),
+              formatName: ref('excluded.formatName'),
+              formatLongName: ref('excluded.formatLongName'),
+              pixelFormat: ref('excluded.pixelFormat'),
             })),
           ),
       );
@@ -238,12 +239,12 @@ export class AssetRepository {
           .values(keyframes)
           .onConflict((oc) =>
             oc.column('assetId').doUpdateSet(({ ref }) => ({
-              pts: ref('asset_keyframe.pts'),
-              accDuration: ref('asset_keyframe.accDuration'),
-              ownDuration: ref('asset_keyframe.ownDuration'),
-              totalDuration: ref('asset_keyframe.totalDuration'),
-              packetCount: ref('asset_keyframe.packetCount'),
-              outputFrames: ref('asset_keyframe.outputFrames'),
+              pts: ref('excluded.pts'),
+              accDuration: ref('excluded.accDuration'),
+              ownDuration: ref('excluded.ownDuration'),
+              totalDuration: ref('excluded.totalDuration'),
+              packetCount: ref('excluded.packetCount'),
+              outputFrames: ref('excluded.outputFrames'),
             })),
           ),
       );

server/test/medium/specs/repositories/asset.repository.spec.ts

@@ -23,6 +23,46 @@ beforeAll(async () => {
   defaultDatabase = await getKyselyDB();
 });
 
+// Metadata extraction is repeatable: probing improves, files get repaired,
+// and a re-run has to be able to correct what an earlier run stored.
+const audioRow = (assetId: string, n: number) => ({
+  assetId,
+  bitrate: 100_000 + n,
+  index: n,
+  profile: n,
+  codecName: `codec-${n}`,
+});
+
+const videoRow = (assetId: string, n: number) => ({
+  assetId,
+  bitrate: 200_000 + n,
+  frameCount: 300 + n,
+  timeBase: 600 + n,
+  index: n,
+  profile: n,
+  level: n,
+  colorPrimaries: n,
+  colorTransfer: n,
+  colorMatrix: n,
+  dvProfile: n,
+  dvLevel: n,
+  dvBlSignalCompatibilityId: n,
+  codecName: `vcodec-${n}`,
+  formatName: `format-${n}`,
+  formatLongName: `format long ${n}`,
+  pixelFormat: `pixfmt-${n}`,
+});
+
+const keyframeRow = (assetId: string, n: number) => ({
+  assetId,
+  pts: [n],
+  accDuration: [n],
+  ownDuration: [n],
+  totalDuration: 1000 + n,
+  packetCount: 10 + n,
+  outputFrames: 20 + n,
+});
+
 describe(AssetRepository.name, () => {
   describe('getTimeBucket', () => {
     it('should order assets by local day first and fileCreatedAt within each day', async () => {
@@ -80,6 +120,90 @@ describe(AssetRepository.name, () => {
   });
 
   describe('upsertExif', () => {
+    it('should replace stored audio metadata on a second extraction', async () => {
+      const { ctx, sut } = setup();
+      const { user } = await ctx.newUser();
+      const { asset } = await ctx.newAsset({ ownerId: user.id });
+
+      await sut.upsertExif({
+        exif: { assetId: asset.id, description: 'first' },
+        audio: audioRow(asset.id, 2),
+        lockedPropertiesBehavior: 'skip',
+      });
+      await sut.upsertExif({
+        exif: { assetId: asset.id, description: 'second' },
+        audio: audioRow(asset.id, 1),
+        lockedPropertiesBehavior: 'skip',
+      });
+
+      await expect(
+        ctx.database.selectFrom('asset_audio').selectAll().where('assetId', '=', asset.id).executeTakeFirstOrThrow(),
+      ).resolves.toEqual(audioRow(asset.id, 1));
+    });
+
+    it('should replace stored video metadata on a second extraction', async () => {
+      const { ctx, sut } = setup();
+      const { user } = await ctx.newUser();
+      const { asset } = await ctx.newAsset({ ownerId: user.id });
+
+      await sut.upsertExif({
+        exif: { assetId: asset.id, description: 'first' },
+        video: videoRow(asset.id, 2),
+        lockedPropertiesBehavior: 'skip',
+      });
+      await sut.upsertExif({
+        exif: { assetId: asset.id, description: 'second' },
+        video: videoRow(asset.id, 1),
+        lockedPropertiesBehavior: 'skip',
+      });
+
+      await expect(
+        ctx.database.selectFrom('asset_video').selectAll().where('assetId', '=', asset.id).executeTakeFirstOrThrow(),
+      ).resolves.toEqual(videoRow(asset.id, 1));
+    });
+
+    it('should replace stored keyframe metadata on a second extraction', async () => {
+      const { ctx, sut } = setup();
+      const { user } = await ctx.newUser();
+      const { asset } = await ctx.newAsset({ ownerId: user.id });
+
+      await sut.upsertExif({
+        exif: { assetId: asset.id, description: 'first' },
+        keyframes: keyframeRow(asset.id, 2),
+        lockedPropertiesBehavior: 'skip',
+      });
+      await sut.upsertExif({
+        exif: { assetId: asset.id, description: 'second' },
+        keyframes: keyframeRow(asset.id, 1),
+        lockedPropertiesBehavior: 'skip',
+      });
+
+      await expect(
+        ctx.database.selectFrom('asset_keyframe').selectAll().where('assetId', '=', asset.id).executeTakeFirstOrThrow(),
+      ).resolves.toEqual(keyframeRow(asset.id, 1));
+    });
+
+    // A probe that could not read a stream sends no object at all, and that must
+    // not be read as "delete what is already known".
+    it('should leave stored media metadata alone when an extraction omits it', async () => {
+      const { ctx, sut } = setup();
+      const { user } = await ctx.newUser();
+      const { asset } = await ctx.newAsset({ ownerId: user.id });
+
+      await sut.upsertExif({
+        exif: { assetId: asset.id, description: 'first' },
+        audio: audioRow(asset.id, 2),
+        lockedPropertiesBehavior: 'skip',
+      });
+      await sut.upsertExif({
+        exif: { assetId: asset.id, description: 'second' },
+        lockedPropertiesBehavior: 'skip',
+      });
+
+      await expect(
+        ctx.database.selectFrom('asset_audio').selectAll().where('assetId', '=', asset.id).executeTakeFirstOrThrow(),
+      ).resolves.toEqual(audioRow(asset.id, 2));
+    });
     it('should append to locked columns', async () => {
       const { ctx, sut } = setup();
       const { user } = await ctx.newUser();